• CynicalTyler (unregistered)

    I'm not sure which is worse: the line of code or that the comment includes an emoticon...

  • Kuba (unregistered) in reply to Anrs
    Anrs:
    Fred:
    So what's a better way of storing the binary representation of a float into a byte array?
    memcpy(&x[4], &value, sizeof value);

    This should also save you from some crashes due to unaligned access.

    While this is probably acceptable on machines which can at least run contemporary Unix (even if it's uclinux), on "tiny" embedded CPUs (think 16kB code EEPROM) it's quite different.

    On C compilers for those CPUs, memcpy may be an intrinsic function that the compiler can generate code for inline when applicable (when size is "small"). If so, then you're home free.

    When memcpy isn't a compiler instrinsic, then quite likely the mere function setup+call+return may eat up more cycles than the actual copying of the memory in question. Never mind the time spent inside of memcpy. I know that's the case on Z8 Encore!, using ZDS II 4.10.x C compiler, large model, static frames.

    Cheers, Kuba

  • (cs) in reply to Thief^
    Thief^:
    Fred:
    So what's a better way of storing the binary representation of a float into a byte array?
    *((float*)(&x[4])) = value; maybe?

    This can blow up on signaling NaNs, depending on the current process-specific "what should I do on signaling NaNs?" behavior.

    Which can be changed by any .dll you load into your process.

    http://blogs.msdn.com/oldnewthing/archive/2008/07/03/8682463.aspx

  • (cs)

    Obviously, the real wtf is that he used unsigned int instead of uint32_t. How can you guarantee that unsigned int is 32-bit?

  • Franz Kafka (unregistered)

    this just goes to show that when you look at your own code from 4 months ago, it may have been written by someone else. Good reason for decent comments.

  • (cs) in reply to akatherder
    akatherder:
    And thus begins the long road to a schizophrenic breakdown.
    $ svn log
r3 | tdurden | Thu, 18 Jul 2002 18:03:46 -0500 | 10 lines
r2 | tdurden | Mon, 15 Jul 2002 17:47:57 -0500 | 1 line
r1 | tdurden | Mon, 15 Jul 2002 17:40:08 -0500 | 34 lines
    EDIT: dangit, why is it double-spacing this?
  • the amazing null (unregistered)

    with any luck, someone will change the hardware hell runs on and that the array that circle was forced into will overflow into a nicer area...

  • moz (unregistered) in reply to Dark Shikari
    Dark Shikari:
    Obviously, the real wtf is that he used unsigned int instead of uint32_t. How can you guarantee that unsigned int is 32-bit?
    The same way you guarantee that uint32_t exists. You test it.
  • dfan (unregistered)

    Here is my own story along the same lines: http://www.dfan.org/writing/comment.html

  • (cs) in reply to cowboy_k
    cowboy_k:
    akatherder:
    And thus begins the long road to a schizophrenic breakdown.
    $ svn log
r3 | tdurden | Thu, 18 Jul 2002 18:03:46 -0500 | 10 lines
r2 | tdurden | Mon, 15 Jul 2002 17:47:57 -0500 | 1 line
r1 | tdurden | Mon, 15 Jul 2002 17:40:08 -0500 | 34 lines
    EDIT: dangit, why is it double-spacing this?
    The first rule of WTF Club is: you do not talk about WTF Club. The second rule of WTF Club is: you do not talk about WTF Club. .. 8th rule: If this is your first night in the WTF Club, you have to write your own WTF!
  • (cs) in reply to jspenguin
    jspenguin:
    java.lang.Chris;:
    jspenguin:
    I've done that before. I needed to send a float over a network connection, so I used something like that to pack it into a 32-bit int. GCC complains with the warning "dereferencing type-punned pointer breaks strict aliasing rules". I ignored it, since it normally works, but when I put the code in an inline function and turned optimization on, it stopped working. The proper way is to use a union: 
    inline uint32_t pack_float(float f) {
    union {
        float flt;
        uint32_t intg;
    } convert;
    convert.flt = f;
    return convert.intg;
}

    No pesky warnings, and with optimization, it compiles to the same code.

    And it wont work unless you always use it to transmit data between the same platforms. Consider what happens when endianness or floating point formats differ ... The correct solution would be to use XDR or ASN.1.

    I forgot to add that I converted the resulting uint32_t to network byte order. So, the de facto protocol spec says, "A IEEE 754 single precision floating point number in network byte order". Almost every processor with an FPU uses IEEE 754. On processors that don't have an FPU, the floating point emulation that GCC links in uses IEEE 754.

    This protocol is a real-time protocol for games -- strictly UDP. Using XDR would be overkill for this.

    I admit I was being a bit pedantic with my "floating point formats differ" comment, as the only platform I've ever used that doesn't support IEEE floating point is the Vax - and I don't think I'll be programming for one of those again. I also had a feeling that if you knew enough to use unions properly, then you'd also ensure network byte order elsewhere in your code. My comments were more of a heads up in case anyone used your example but then wondered why their x86 machine couldn't transmit floats to a SPARC or PowerPC machine. Personally, I'd still use XDR, but then I've got a wrapper library that makes it easy.

  • Ghost (unregistered)

    What if that line of code was in the initial import that rjacobs did? The only wrongdoing we would be guilty of then, would be importing it...

  • ItIsntAWTF (unregistered)

    That really isn't a WTF. That's a reasonable way to do that. You're changing the type for the benefit of the compiler. There's nothing wrong with doing that. A beginner or mid-level C programmer might not see what it's doing, but it is the correct way to tell the compiler "treat these four bytes as ..." Everything is bytes anyway. Types exist only for the compiler and the programmer.

    The thing to do is to have a simple comment that says, "this treats these bytes of float as an array of char", and that's it.

    JBoss Seam training

  • Mate (unregistered) in reply to jspenguin

    This, of course, makes the assumption that the sending and receiving machines have same byte order (htonl(3) and ntohl(3) help with that), and also that the machines have same sort of floating point representation, which might actually be a safer guess since I suppose most machines have IEEE floating point numbers... But if they haven't, I suppose you could implement a function that converts between different FP representations.

  • (cs) in reply to Mate
    Mate:
    and also that the machines have same sort of floating point representation, which might actually be a safer guess since I suppose most machines have IEEE floating point numbers... But if they haven't, I suppose you could implement a function that converts between different FP representations.
    FWIW, Microchip invented their own float format for the PIC, which was then imported into the CCS 'not-exactly-real-C' compiler. I have done those FP conversion functions!
  • Bobo The Sperm Whale (unregistered)

    Actually the code is pretty good at converting a float-y type into an int-y type, which may be necessary if you need to bit-twiddle a float. Perhaps I'm only posting this because I've written something similar. ;)

  • (cs) in reply to Kuba
    Kuba:
    While this is probably acceptable on machines which can at least run contemporary Unix (even if it's uclinux), on "tiny" embedded CPUs (think 16kB code EEPROM) it's quite different.

    On C compilers for those CPUs, memcpy may be an intrinsic function that the compiler can generate code for inline when applicable (when size is "small"). If so, then you're home free.

    When memcpy isn't a compiler instrinsic, then quite likely the mere function setup+call+return may eat up more cycles than the actual copying of the memory in question. Never mind the time spent inside of memcpy. I know that's the case on Z8 Encore!, using ZDS II 4.10.x C compiler, large model, static frames.

    Somewhere around half-way through your post you got confused and exchanged the roles of space and time in your argument, Einstein... Just how many cycles of RAM does that Z8 Encore have, anyway?
  • Steve (unregistered)

    Any code you've written more than six months ago may as well have been written by someone else.

  • The Fake WTF (unregistered)

    In the future, all coders will be required to think THREE times.

    And if that doesn't work, we'll send you to a mandatory training class.

  • (cs) in reply to Ikke
    Ikke:
    Please think twice before reading this post.
    Please think twice before quoting this post.
  • JimBob (unregistered) in reply to Fred

    float x = 1; char * v = (char *) x; v[0]..v[8]

  • anttirt (unregistered)

    The only legal (ie. not undefined behavior) way to do this is to cause N = sizeof(float) char or unsigned char objects to alias a pre-existing float object.

    What this means in practice:

    float f = ...;
char* p = (char*)&f; 
/* work with p[i] */

    While the union "cast" works in practically all compilers, it's still undefined behavior according to the standard, and so are all the other "clever" ways of causing aliasing. For further information, refer to ISO/IEC 9899:1990 and http://www.cellperformance.com/mike_acton/2006/06/understanding_strict_aliasing.html

  • JimBob (unregistered) in reply to JimBob
    JimBob:
    float x = 1; char * v = (char *) x; v[0]..v[8]
    d'oh. v[0]..v[7]

    or v[0]..v[3], depending on architecture.

  • JimBob (unregistered) in reply to JimBob
    JimBob:
    float x = 1; char * v = (char *)& x; v[0]..v[8]
    d'oh. v[0]..v[7]

    or v[0]..v[3], depending on architecture.

  • Vic (unregistered)

    I loved it!

    Since this is true confessions...

    Anytime I take a look-see at my code, I find at least 1 WTF-y line in it. I'll even comment 'This looks WTF and is done because...'

    I envy those who have the time/resources/ability/scope/authority/etc to plan each line of code to perfection.

  • Alex (unregistered) in reply to Maks Verver
    Maks Verver:
    I'd like to point out that of the various methods given in the reactions, Anrs method is one of the best: 
        void store(char *a, float f) {
        memcpy(&a, &f, sizeof(float));
    }
    As unlike the other functions listed, Only Anrs successfully annihilates the stack around its first parameter.

    Fixed.

    (Actually, so does John's, but he's already been corrected ;)

  • M. (unregistered)

    This sort of thing happens to me everytime I come back from vacation. "WTH wrote this crap!?". Well - I did.

  • asdf (unregistered) in reply to Fred

    Better way? Store it as a float and memcpy the chars into right place. That'll get you around alignment stuff and other unpredictable behaviour. Plus it doesn't look half as bad as that.

  • Godot (unregistered)

    Wow, just wow! Makes me remember what I dislike about C...

  • (cs) in reply to Ken B
    Ken B:
    troels:
    argh!
    I use "argv" and "argc" all the time. What's "argh"?
    argc is your argument count argv is the vector containing your arguments argh is a hashmap containing your arguments, in case the user has typed them in in the wrong order yet again.
  • Eric D (unregistered)

    This is actually a very good example to show C programming students with the question: "How can this work most of the time ?"

  • (cs)

    Of course, where I'm at right now they switched from one source control repository to another after primary development was complete, so most witch hunts end there...

  • CS (unregistered) in reply to Maks Verver
    Maks Verver:
    I'd like to point out that of the various methods given in the reactions, Anrs method is one of the best: 
        void store(char *a, float f) {
        memcpy(&a, &f, sizeof(float));
    }

    D'oh! So nearly...

    Unfortunately, &a needs to be amended to a I think :)

    Other than that I agree this is the tidiest way in general, though given the context of the original post, I'd be tempted to use:

    struct {
  uint16 partTheFirst;
  uint16 partTheSecond; // Or whatever preceded the float to give it an offset of 4
  float floatValue;
  // ...
} packet;

packet.floatValue = value;

doByteyStuffWith((char*)&packet);
  • Rob Jacobs (unregistered)

    Hey! I recognise that line of code! Shame I didn't think twice before I wrote it - maybe even thinking once would have been better?

  • Andrew Beard (unregistered) in reply to Alex
    Alex:
    Maks Verver:
    I'd like to point out that of the various methods given in the reactions, Anrs method is one of the best: 
        void store(char *a, float f) {
        memcpy(&a, &f, sizeof(float));
    }
    As unlike the other functions listed, Only Anrs successfully annihilates the stack around its first parameter.

    Fixed.

    (Actually, so does John's, but he's already been corrected ;)

    I tried to follow this, but I don't quite get it. Anrs solution seems to work great, it's Maks' that shits the bed. Offset of 4 aside, the first parameter to memcpy in the store function case is going to be a pointer to the address of the destination, not the the address of the destination itself. Shouldn't this read:

    memcpy(a + 4, &f, sizeof(float));

    Pointer type in the first and second parameters.

    Anrs' worked fine though, as &x[4] is already a pointer type. Is there something wrong with Arns' code that I'm missing, or was it the conversion of Arns' memcpy call to a function that screwed it up?

  • (cs) in reply to Andrew Beard

    Its easy to accidently check in stuff under a different name. If you have a name that does builds or a checkout script that uses a name and password, svn remembers the last action credentials. So if you use tortoise or something to commit, it commits the code under that name, which may not be your name.

  • kudasai master (unregistered) in reply to Maks Verver
    Maks Verver:
        void store(char *a, float f) {
        memcpy(&a, &f, sizeof(float));
    }

    You probably mean:

        void store(char *a, float f) {
        memcpy(a, &f, sizeof(float));
    }
  • Worf (unregistered) in reply to Kuba
    Kuba:
    When memcpy isn't a compiler instrinsic, then quite likely the mere function setup+call+return may eat up more cycles than the actual copying of the memory in question. Never mind the time spent inside of memcpy. I know that's the case on Z8 Encore!, using ZDS II 4.10.x C compiler, large model, static frames.

    That's when you get around to the "optimization" phase of a project. As far as anything is concerned, a proper functioning memcpy() will work regardless of the original types, alignment (source, destination, or host architecture), or other issue (even handling overlapping memory regions).

    A programmer should use memcpy() rather than their own copy routines. If the memcpy() is implemented inside a performance critical area, then perhaps it's something that can be optimized later.

    But if the code doing the packing/unpacking is only used to ship data off a wire, that's not performance critical, the convenience of knowing that memcpy will "get it right" versus the overhead of doing it yourself (especially months later when some bug or fix has to be applied that changes an underlying data structure) mantenance...

    Plus, you avoid code that looks like the article. Would you rather see a memcpy() or the article code when tasked with finding a bug?

  • The Masked Engineer (unregistered)

    Aliasing violation. These days your compiler should have a good 'ol whine about that.

  • (cs) in reply to troels
    troels:
    In an application I was maintaining I found the well-intentioned statement:

    buffer[strlen(buffer)] = '\0';

    argh!

    Could be worse.

    The following is lifted from the Quake3 source code, trimmed and altered for brevity:

      char* text = "some text to parse";
  char* text_p;

  text_p = text;
  while(1)
  {
    token = COM_Parse( &text_p );
    if (!token)
      break;
  }

    Where COM_Parse takes a char** that will slide over the text as it is parsed into tokens and returns a char* to an internal buffer containing the current token. If no more tokens are available it returns said buffer filled with only a null terminator.

    Subtle, but quite destructive.

  • (cs) in reply to anttirt
    anttirt:
    While the union "cast" works in practically all compilers, it's still undefined behavior according to the standard,
    You're OK using it in GCC: the compiler explicitly guarantees to make it work, as an extension to the C spec.
  • (cs) in reply to ItIsntAWTF
    ItIsntAWTF:
    That really isn't a WTF.
    Sigh, it so is.
    ItIsntAWTF:
    That's a reasonable way to do that.
    Sigh, it so isn't.
    ItIsntAWTF:
    You're changing the type for the benefit of the compiler.
    No, you're changing it to the detriment of the compiler.
    ItIsntAWTF:
    There's nothing wrong with doing that.
    Nothing wrong with misleading the compiler? I rather think there is.
    ItIsntAWTF:
    A beginner or mid-level C programmer might not see what it's doing,
    LOL, that's beautiful irony. So, tell me, what level C programmer do YOU think you are?
    ItIsntAWTF:
    but it is the correct way to tell the compiler "treat these four bytes as ..." Everything is bytes anyway. Types exist only for the compiler and the programmer.
    So near... yet so very very far. Yes, types do exist only for you and the compiler. And you know what they do to the compiler? They change the code it generates. Yep, that's right. It doesn't just check the types match up in assignments and function calls and then throw them away. Not at all.
    ItIsntAWTF:
    The thing to do is to have a simple comment that says, "this treats these bytes of float as an array of char", and that's it.

    JBoss Seam training

    ^^^^^^^^^^^^^^^^^^^^^ PUBLIC SERVICE ANNOUNCEMENT:

    We would like to warn our readers that posting misleading and incorrect advice on TDWTF in a way that demonstrates very clearly that you do not understand compiler technology is NOT A GOOD WAY TO ADVERTISE YOUR TRAINING COURSE.

    Here's a google term for you: "Type-based alias analysis". It's not just a theoretical argument. Ignore it, and your code WILL be invalid.

    Oh, p.s.: Exactly how does casting something to (unsigned int *) and then dereferencing it turn it into "an array of char"?

  • James D (unregistered) in reply to OperatorBastardusInfernalis

    That's a deliberate WTF joke about the undefined behavior present in writing into an int field of a union and then reading from a float field? Fair enough, if so: this is one of the most common bugs/misunderstandings among intermediate C and C++ programmers. Type punning is allowed in only very limited cases, and int/float isn't one of those cases.

  • Gur (unregistered)

    I'd say, if speed is your primary concern, you're likely to run into more of these situations. for example: float a, b; if (((int)&a) > ((int)&b))

  • jpm (unregistered)

    Ain't nothing unusual with developers writing stuff like that when they're new to commercial software development. Most developers starting out do that (complete with smileys in the comments) because they think it'll impress. As they get older they figure out the bigger picture and stop doing it (as Rob did here).

    The next level of experience might say that the WTF here is how come a less experience developer was able to get code like that into the code-base without it being picked up in a peer review?

    Then you learn that because project tasks like design, CM, code review and testing don't actually provide marketing with new features, they're seen as obstructions to getting a product out on time so 'they will get done later...'

  • Rhialto (unregistered) in reply to Worf
    Worf:
    As far as anything is concerned, a proper functioning memcpy() will work regardless of the original types, alignment (source, destination, or host architecture), or other issue (even handling overlapping memory regions).
    For (potentially) overlapping memory regions, there is memmove(3).
  • (cs)

    Ah, the tangled old code we [try to] forget...

  • RF (unregistered) in reply to ParkinT
    ParkinT:
    THAT IS UNCONSCIONABLE! That another developer would check in a change under Rob Jacobs' name!

    It was the code fairy - taking good code and leaving a dollar... and bad code.

  • Hans (unregistered) in reply to ParkinT

    I love reading WTF for exactly this kind of thing. I have done the same exact thing. Internally accusing someone of some infraction only to review the logs and find it was me. You made my morning.

    Hans

  • YodaWise (unregistered)

    Is anyone not going to comment on the difference between: memcpy(x, &value, sizeof(value)); and *(unsigned int *)&x[4] = ...

    The first stores the value in x[0], x[1], x[2] and x[3]; The second stores the value in x[4], x[5], x[6] and x[7].

Leave a comment on “Please Think Twice”

Log In or post as a guest

Replying to comment #:

Log Out

« Return to Article