Bad casting choice



  • While helping another team in my company building an x86_64 version of some internal tools, I found this interesting antipattern :

    struct foo { 

    char* name;

    /* more fields */

    };

    char *foo_get_name(struct foo * fp) {

    return (int) fp->name;

    }

    This is not an isolated mistake, there are around 700 occurences of this pattern (casting char* to int to return char*).

    WTF ?

     



  • Wow that's bad. But I think the real wft is c, because the code is valid.

    How can you have a typed language which implicit cast an int to a pointer. (I had to start gcc just to test if this is really valid c code. It is).




  • @mt@ilovefactory.com said:

    How can you have a typed language which implicit cast an int to a pointer.
     

    You are pushing the definition a bit calling C a typed language.

    It has type declarations, but the only difference between them is size and if they are integral or floating point. Just like assembly, what souldn't be a surprize.



  • MSVC 2013:

    1>bad_casting_choice.c(7): warning C4047: 'return' : 'char *' differs in levels of indirection from 'int'
    ========== Build: 0 succeeded, 1 failed, 0 up-to-date, 0 skipped ==========
    

    GCC 4.8.2:

    bad_casting_choice.c: In function ‘foo_get_name’:
    bad_casting_choice.c:7:9: warning: cast from pointer to integer of different size [-Wpointer-to-int-cast]
      return (int) fp->name;
             ^
    bad_casting_choice.c:7:2: warning: return makes pointer from integer without a cast [enabled by default]
      return (int) fp->name;
      ^

    I will admit that I don't know how to use MSVC, so it may have a switch somewhere that enables nicer error messages (or a switch somewhere that I inadvertently disabled while trying to get it to compile a .c file without a precompiled header.)

    But either way, both of the compilers I tried with default options noticed that something is wrong. Actually, MSVC noticed a problem before I even compiled the code (while I was trying to paste it in):

    So, gvh, how many warnings does the project spit out when you compile it?



  • Yes it is valid C, but the behavior is implementation defined which is somehow worse.



  • @delta534 said:

    Yes it is valid C, but the behavior is implementation defined which is somehow worse.

    It is valid and the behaviour is specification defined. The two conversions are implementation-defined, but specification requires that pointer via sufficiently large integral type round-trips.

    On 64-bit platforms, int is not a sufficiently large integral type (and recent gcc won't compile it there).



  • @Ben L. said:

    So, gvh, how many warnings does the project spit out when you compile it?
     

    Good point. I don't how many warning there were before I modified the makefiles to use a nice set of -W options to gcc.

    At first build, there were nearly 30000 warnings.

    Lots of "implicit declaration of ..." (missing includes)

    Lots of unused variables (there are dozens of files where each fonction starts with an "int i;" declaration)

    And of course, thousands of warning about pointers, integer size, ...


  • Considered Harmful

    I was expecting this thread to be about Ben Affleck as Batman.


  • Discourse touched me in a no-no place

    @joe.edwards said:

    I was expecting this thread to be about Ben Affleck as Batman.
    Courtney Love as Mary Poppins?



  • @gvh said:

    Lots of unused variables (there are dozens of files where each fonction starts with an "int i;" declaration)

    Wow. Cut/Paste functions? "Well, it compiles, and when I run it I don't get a segfault, so it must be OK. Even with 30,000 warnings; they can't really mean anything..."



  • @dkf said:

    @joe.edwards said:
    I was expecting this thread to be about Ben Affleck as Batman.
    Courtney Love as Mary Poppins?

    Alec Baldwin as Ghandi?



  • Sadly, we will be seeing WTFs like this into the 2060s because C will never just die already.



  • @morbiuswilters said:

    Sadly, we will be seeing WTFs like this into the 2060s because C will never just die already.

    This, so very very much. With all the security issues, memory leaks etc you would think by now most modern software would be written using managed code or something that makes it impossible for a out of bounds index to give you root control over the computer. But no.

    Just look at Objective C. Whopee, another C language etching into the skull of a new generation, ensuring that in 20 years, someone will reinvent the garbage collector, call it something new and be hailed as a genius by their fellow idiots.



  • @arh said:

    Just look at Objective C. Whopee, another C language etching into the skull of a new generation, ensuring that in 20 years, someone will reinvent the garbage collector, call it something new and be hailed as a genius by their fellow idiots.

    He guys I have an idea! We have a garbage collector in Obj C but it just counts references right? What if would just follow all pointers from the currently running code and delete everything that isn't reachable! That way we can detect unreachable objects which still reference each other!



  • @dtech said:

    He guys I have an idea! We have a garbage collector in Obj C but it just counts references right? What if would just follow all pointers from the currently running code and delete everything that isn't reachable! That way we can detect unreachable objects which still reference each other!

    Terrible idea; it'll break all my xor linked lists.



  • @arh said:

    @morbiuswilters said:
    Sadly, we will be seeing WTFs like this into the 2060s because C will never just die already.

    This, so very very much. With all the security issues, memory leaks etc you would think by now most modern software would be written using managed code or something that makes it impossible for a out of bounds index to give you root control over the computer. But no.

    Oh, come on. Name one example of a popular package where poor boundary checks lead to a CERT alarm every couple of weeks.

     Other than OpenSSL, that is.

     



  • @Severity One said:

    Other than OpenSSL, that is.

    We need you at the new site.



  • @arh said:

    @morbiuswilters said:
    Sadly, we will be seeing WTFs like this into the 2060s because C will never just die already.

    This, so very very much. With all the security issues, memory leaks etc you would think by now most modern software would be written using managed code or something that makes it impossible for a out of bounds index to give you root control over the computer. But no.

    Just look at Objective C. Whopee, another C language etching into the skull of a new generation, ensuring that in 20 years, someone will reinvent the garbage collector, call it something new and be hailed as a genius by their fellow idiots.

    I want the D.


  •  @morbiuswilters said:

    Sadly, we will be seeing WTFs like this into the 2060s because C will never just die already.
    Well, at least some libcs have begun adjusting time_t to be an int64 instead of waiting until [url=http://en.wikipedia.org/wiki/Year_2038_problem]y2038[/url].



  • Classic rookie mistake. Looking up the address of the structure member in the self-comparison is one of the first patterns you should learn:

    return &(fp != fp)[fp->name];

    That way you don't get any warnings.



  •  Wow that is really really bad


Log in to reply