Opened 6 years ago

Closed 5 years ago

#10224 closed bug (no change required)

Changed some dynamic casts to reinterpret casts

Reported by: Ezodev Owned by: anevilyak
Priority: normal Milestone: Unscheduled
Component: Applications/Debugger Version: R1/Development
Keywords: gci2013 Cc:
Blocked By: Blocking:
Has a Patch: yes Platform: All

Description

These dynamic casts wern't checked. Reinterpret cast is instant, dynamic cast is slow. If there is no error checking, dynamic cast gives nothing. It will just cause application crash.

Attachments (4)

Change History (15)

comment:1 Changed 6 years ago by Ezodev

Has a Patch: set

comment:2 Changed 6 years ago by umccullough

Per Ezo in IRC, this should resolve CIDs 611227 and 611228 (and possibly others)

comment:3 Changed 6 years ago by bonefish

The cast operator you're looking for is static_cast, not reinterpret_cast.

The unchecked dynamic_casts are really intended, though. In all cases the actual type of the object is ensured by other means. A mismatch would be a program error and the immediate crash caused by the NULL pointer would be desirable (a static_cast to a wrong type wouldn't necessarily cause an immediate crash). Performance isn't much of a concern in these cases. I don't know, maybe adding an ASSERT would be cleaner. Or alternatively dereferencing the original pointer and casting the reference (causing an exception on type mismatch).

Changed 6 years ago by Ezodev

new patch

comment:4 Changed 6 years ago by Ezodev

If speed isn't critical, I've added null checking. It's better than crash few lines later, It will call destructors etc. Error code 2 because it's unused and someone can easily track problem in source. And it will silient off tools to static analysis.

Last edited 6 years ago by Ezodev (previous) (diff)

comment:5 Changed 6 years ago by bonefish

A call to debugger() would be much better than exit()ing in those cases, ideally with a way to recover afterward, if that doesn't need more than a simple return.

As per our coding style guide there should be a space between if and (.

comment:6 Changed 6 years ago by Ezodev

I've added calls to debugger(). But I don't understand what you means by way to recover afterward. If this is simply returning from function, I don't know where can I do it without problems.

Changed 6 years ago by Ezodev

comment:7 Changed 6 years ago by Ezodev

Ups. new patch don't compiling, sorry.

Last edited 6 years ago by Ezodev (previous) (diff)

Changed 6 years ago by Ezodev

comment:8 Changed 6 years ago by Ezodev

Fixed. Is this solution good?

comment:9 Changed 6 years ago by bonefish

  • Don't use fprintf(). Just call debugger() in all cases. And return afterward.
  • String literals should not be wrapped like this. Close the literal at the end of the line and start a new on the next line (indent as usual with one additional tab), like:
    printf("this line is "
    	"too long\n");
    
  • Null pointer checks should be spelled explicitly: foo == NULL.

comment:10 Changed 5 years ago by luroh

Milestone: R1Unscheduled

Moving Debugger related tickets out of R1 milestone (Prop #14).

comment:11 Changed 5 years ago by anevilyak

Resolution: no change required
Status: newclosed
Note: See TracTickets for help on using tickets.