• (cs) in reply to Manni
    Manni:

    Since everyone is so focused on other aspects of this post, here's a very minor WTF, but one that gets under my skin regardless:

    stopHere = stopHere + (bignumberjustincase);

    Could he really not know about the += operator?



    is there any reason to put (bignumberjustincase) instead of bignumberjustincase
  • Anonymous Coward (unregistered) in reply to RevMike

    So tell me, what does vector1 * vector2 evaluate to?

    The same thing as string1 * string2.

  • (cs) in reply to Anonymous Coward
    Anonymous:
    So tell me, what does vector1 * vector2 evaluate to?

    The same thing as string1 * string2.


    string1 * string2 invoke the "concatenate and run through Swedish Chef filter" which obviously doesn't make any sense for numeric vectors. 

    Sorry, try again.

  • jdt (unregistered) 
      char temporary_file_name[ 19 ];
      char * temorary_file_name_name ;

      strcpy( temporary_file_name, "thefileXXXX" );
      temorary_file_name_name = _mktemp( temporary_file_name )


    With code this bad, I guess it's no surprise that he didn't know he could just declare char temp_file_name[] = "Isuckatstrings";.
    And I don't think it's random chance that he made the character array 19 bytes long, since the string "temporary_file_name" is exactly 19 characters long.  The code probably had strcpy( temporary_file_name, "temporary_file_name") in it at first, until it just wouldn't work for lack of a string terminator.  Not that I'd expect him to know about such things...  Bonus points for stupidity if it actually worked for awhile until he added the line temporary_file_name_name = _mktemp( temporary_file_name ), because that wonderfully named_named variable could have been acting as an accidental string terminator until overwritten with a non-zero value.
  • Mike N (unregistered) in reply to Zlodo
    Zlodo:
    Since he declares variables in between statements, this is C++.


    Not necessarily; newer versions of GCC allow that.
  • Anomalous Cowherd (unregistered) in reply to Alun Jones
    Anonymous:

    Anonymous:

    snprintf returns the number of characters that would have been written had the buffer been large enough.

    size_t sz = snprintf(NULL, 0, fmt, ...) + 1;
    char *s = malloc(sz);

    if (s)
       snprintf(s, sz, fmt, ...);

    Many platforms provide asprintf(), which does the above for you.

    I guess it depends whose snprintf you use.  glibc 2.0, for instance, just returns -1 if the buffer isn't big enough.  Sweet.

    The Microsoft snprintf documentation says it returns "a negative value" (can this be used as a random number generator?)



    Sure enough. I didn't realize that any currently-shipping implementations got that wrong. glibc was fixed a long time ago, but a quick glance at the current MSDN docs shows exactly what you described.
  • Garcimore (unregistered)

    Reminds of a code I saw a long time ago. It was a multi threaded application and the guy
    used a lot of temp files to pass data from on thread to another. Of course, without mutex or semaphore :o

    He also had a design pattern of its own :


    if(!condition)
    {
    }
    else
    {
        statements;
        [...];
    }

  • (cs) in reply to Anomalous Cowherd
    Anomalous Cowherd:


    size_t sz = snprintf(NULL, 0, fmt, ...) + 1;
    char *s = malloc(sz);

    if (s)
       snprintf(s, sz, fmt, ...);

    Many platforms provide asprintf(), which does the above for you.


    Ah good, a function that conveniently allocates some memory for you.  Putting such a function into the hands of a programmer like today's shining star, who has a tenuous grasp of resource management on a good day, can only lead to hilarity.

    At best, he'll just leak some memory.

    At worst, he'll cause problems that make overfull directories look like good design.

    Oh, and WTF is with this forum software?  Is it too much to ask to be able to quote people's names consistently??

  • moocat (unregistered) in reply to Mike N
    Anonymous:
    Zlodo:
    Since he declares variables in between statements, this is C++.


    Not necessarily; newer versions of GCC allow that.

    It's a C99 thing.

  • (cs) in reply to Manni
    Manni:

    Since everyone is so focused on other aspects of this post, here's a very minor WTF, but one that gets under my skin regardless:

    stopHere = stopHere + (bignumberjustincase);

    Could he really not know about the += operator?



    I disagree with you on this one.  IMHO, this is just a matter of preference and style.  The real WTF is that he does this:
    int stopHere = (long)string_write_to + (int)0;

    is it really needed to cast 0 into an int?





  • (cs) in reply to TankerJoe

    It could be worse. He could have written the entire function in pointer arithmatic (as it is he's 2/3 of the way there). I haven't seen this much of the rarer alcoves of the C standard lib in one place in years. =D

    My only regret is that he didn't use striBignumjustincase.

  • Chuck (unregistered) in reply to Brendan Kidwell

    sort of looks like... a call to a function pointer being cast into a pointer to a string??

  • ack (unregistered)

    I don't believe this is from a real program... How can you?

  • (cs) in reply to TankerJoe
    TankerJoe:
    Manni:

    Since everyone is so focused on other aspects of this post, here's a very minor WTF, but one that gets under my skin regardless:

    stopHere = stopHere + (bignumberjustincase);

    Could he really not know about the += operator?



    I disagree with you on this one.  IMHO, this is just a matter of preference and style.  The real WTF is that he does this:
    int stopHere = (long)string_write_to + (int)0;

    is it really needed to cast 0 into an int?





    There is absolutely no need to do that casting. As for your argument of preference and style, I had no doubt I'd get at least one person to disagree with me. But what you're saying is that it's his preference to write out longer lines of code rather than using shortcuts that simplify the code's appearance. Since it's apparently acceptable to use VB-esque code like this, I say we get rid of the || and && operators.

    if (somecondition) {
     doAction();
    } else if (someothercondition) {
     doAction();
    } else if (yetanothercondition) {
     doAction();
    }

    You're right! It's just as easy to read, and took me only twice as long to type!

  • (cs)

    Everyone's ragging on this guy for the horrible things he's done --- well, that's fair, but to be REALLY fair
    you should credit him for the good things as well.  For example, he doesn't waste any time or lines-of-code
    checking for errors, since we all know that all three of those fopen() calls will always succeed.

    I admit that "bignumberjustincase" is poorly named, though.  Obviously it should be "bigfloatingpointjustincase",
    which was a wise decision in case the newly-created file was larger than 4 Gb.

    ok
    dpm

  • (cs) in reply to dpm

    {} is presumably for setting breakpoints.

  • tecxx (unregistered) in reply to Maurits

    Maurits:
    {} is presumably for setting breakpoints.

     

    afaik you can only set breakpoints at statements, like

    int x = 3;

    (at least in visual studio you can't set breakpoints at whitespace lines or {} )

  • (cs) in reply to tecxx

    df

  • Someone (unregistered)

    "void printf_to_string( int first, char * fomat, ... )"

    O, come on! The fact that someone who probably is not a native speaker makes a simple mistake spelling 'vomit' does not warrant a WTF.

  • (cs) in reply to dabocla

    My browser went nuts on me, sorry about that partial post.

    I am new to C, about two weeks into it.  I think about 90% of the examples I looked at contained the sprintf function for building strings.  I hope that this guy is less experienced in C than I am.

     

  • (cs) in reply to RevMike
    RevMike:
    In other words, when a function is called the stack contains, working from the bottom up, the return address, the last parameter, the second to last parameter, ..., the second parameter, and the first parameter.  In this way, as long as the function can determine at runtime how many times to pop the stack, the function can have a variable number of parameters.  One of the named parameters must indicate, in some way, how many other parameters there are.


    That's not accurate. Aside from the right-to-left push order, the x86 C calling convention also specifies that it's not the callee but the caller who cleans up the stack. And since the caller always knows how many parameters it pushed, it can also pop them again.
    In addition, you've placed the return address incorrectly. It's the last thing to be pushed, after all parameters, and thus comes out on top.

    Here's a typical x86 call to printf:
    printf("Hello %s, this are visitor # %d.\n", visitor, viscount);
    ->
    push viscount
    push visitor
    push stringconstant52523
    call _printf ; pushes return address
    add esp, 12 ; equivalent to three pushs

    Within printf, the stack then looks like this:
    ESP    -> return address
    ESP+4  -> stringconstant52523
    ESP+8  -> visitor
    ESP+12 -> viscount

  • Gn (unregistered) in reply to Chris
    Anonymous:
    I particularly liked his filehandle naming convention:

    FILE1 FILE11 FILE111



    He's using base 1 obviously!

  • nitpicker (unregistered)
    Alex Papadimoulis:

      {} /* make the variable */
      char * string_write_to = va_arg( marker, char * );

    The variable string_write_to is also a major WTF in this deplorable, yet humorous code.  It really is required by the function, yet is passed, or received rather, on the variable argument list.  Furthermore, it's passed between the format string and the parameters needed by the format, er fomat, string.  It should be given its own place in the regular arguments, preferably before fomat (sic).  But then again, we'd end up with <FONT color=#000080>sprintf</FONT> before long and we can't go down that road. [;)]

    Anonymous:
    Not to defend the code, or the technique but I always like finding a bright side of things... I'd like to point out that the general technique has one advantage that sprintf does not: you can allocate exactly the amount of memory that you need to hold an arbitrarily formatted string. sprintf(buf, "%f", f); // How many bytes for buf?

    It's true that this technique could be used to allocate the exact amount of memory needed once the printf formatting has taken place.  It's equally true that this code doesn't take advantage of this ability but can freely write out a bignumberjustincase number of characters that can easily overflow the buffer held at string_write_to.

  • Robin Lavall&#233;e (unregistered) in reply to CornedBee

    Didn't you just summarize what he wrote?
    He said the return address was last on the stack (bottom-up!), so it is the last thing to be pushed!
    A stack usually grows from top to bottom.

    Robin

  • (cs) in reply to CornedBee
    CornedBee:
    RevMike:
    In other words, when a function is called the stack contains, working from the bottom up, the return address, the last parameter, the second to last parameter, ..., the second parameter, and the first parameter.  In this way, as long as the function can determine at runtime how many times to pop the stack, the function can have a variable number of parameters.  One of the named parameters must indicate, in some way, how many other parameters there are.


    That's not accurate. Aside from the right-to-left push order, the x86 C calling convention also specifies that it's not the callee but the caller who cleans up the stack. And since the caller always knows how many parameters it pushed, it can also pop them again.
    In addition, you've placed the return address incorrectly. It's the last thing to be pushed, after all parameters, and thus comes out on top.

    Here's a typical x86 call to printf:
    printf("Hello %s, this are visitor # %d.\n", visitor, viscount);
    ->
    push viscount
    push visitor
    push stringconstant52523
    call _printf ; pushes return address
    add esp, 12 ; equivalent to three pushs

    Within printf, the stack then looks like this:
    ESP    -> return address
    ESP+4  -> stringconstant52523
    ESP+8  -> visitor
    ESP+12 -> viscount



    Thanks.  Its been a LONG time since I needed to do stuff at this level.
  • (cs) in reply to Robin Lavall&#233;e
    Anonymous:
    Didn't you just summarize what he wrote?
    He said the return address was last on the stack (bottom-up!), so it is the last thing to be pushed!
    A stack usually grows from top to bottom.

    Robin


    No, he had it right.  I said the return address was the first thing pushed, so that once all the parameters were popped it would be the next available item from the stack.  I got it wrong, at least as far as x86 C conventions.
  • Eric (unregistered) in reply to Dan
    Anonymous:
    Now, all thats left to do is take this coders face, push it into the monitor showing his code, and say "NO. BAD.".  It's either that, or electric collars...


    This really does look like treatable ignorance, not pathological stupidity.
  • Anonymous Bastard (unregistered) in reply to Kooooch
    Anonymous:

    Finally! Comments that aid in understanding the code!

    // this is required, or it will not work for some reason

    And, oh yeah, "FIRST!"

    Kooooch:

    Well, it was first when I started posting....

    Dude, it took you 7 minutes to type your first post? 

  • Ozru (unregistered) in reply to CornedBee
    CornedBee:
    That's not accurate. Aside from the right-to-left push order, the x86 C calling convention also specifies that it's not the callee but the caller who cleans up the stack. And since the caller always knows how many parameters it pushed, it can also pop them again.
    In addition, you've placed the return address incorrectly. It's the last thing to be pushed, after all parameters, and thus comes out on top.

    Here's a typical x86 call to printf:
    printf("Hello %s, this are visitor # %d.\n", visitor, viscount);
    ->
    push viscount
    push visitor
    push stringconstant52523
    call _printf ; pushes return address
    add esp, 12 ; equivalent to three pushs

    Within printf, the stack then looks like this:
    ESP    -> return address
    ESP+4  -> stringconstant52523
    ESP+8  -> visitor
    ESP+12 -> viscount

    If you're really going to get picky, then you should note that the x86 stack grows down, and thus adding 12 to SP is equivalent to popping three 32-bit values, not pushing them.

  • (cs) in reply to Manni
    Manni:
    TankerJoe:
    Manni:

    Since everyone is so focused on other aspects of this post, here's a very minor WTF, but one that gets under my skin regardless:

    stopHere = stopHere + (bignumberjustincase);

    Could he really not know about the += operator?



    I disagree with you on this one.  IMHO, this is just a matter of preference and style.  The real WTF is that he does this:
    int stopHere = (long)string_write_to + (int)0;

    is it really needed to cast 0 into an int?





    There is absolutely no need to do that casting. As for your argument of preference and style, I had no doubt I'd get at least one person to disagree with me. But what you're saying is that it's his preference to write out longer lines of code rather than using shortcuts that simplify the code's appearance. Since it's apparently acceptable to use VB-esque code like this, I say we get rid of the || and && operators.

    if (somecondition) {
     doAction();
    } else if (someothercondition) {
     doAction();
    } else if (yetanothercondition) {
     doAction();
    }

    You're right! It's just as easy to read, and took me only twice as long to type!



    Just think about how much more money you would make by charging $$ per line of code!!

    Seriously though, I have no idea what the preference is of the screwball who wrote that original piece of crap.  What I am saying is that the difference between...

    value += increase;

    and

    value = value + increase;

    is preference and style, maybe even stupidity.  It is not a WTF, even a minor one.
  • A. Nonymous (unregistered)

    Bah! This is a hoax. Someone's making up WTFs. Unless this has been transcribed by hand from the original with some typos, it would crash every time it ran (notice the second time it closes FILE1, where FILE11 would be more appropriate?). Somehow I don't think that would even make it as far as a code review, even from someone at the level of the person that allegedly wrote this.

  • Now See Here (unregistered) in reply to Garcimore
    Anonymous:
    Reminds of a code I saw a long time ago. It was a multi threaded application and the guy
    used a lot of temp files to pass data from on thread to another. Of course, without mutex or semaphore :o

    He also had a design pattern of its own :


    if(!condition)
    {
    }
    else
    {
        statements;
        [...];
    }


    Yes, We have no bananas.

    Printf to String:

    He's obviously been burned somewhere by trying to write a string of larger than 128 bytes (or some other hard limitation, depending on the compilor).

    And the BigNumberJustInCase is for writing temporary files bigger than 2 gigs.  

    Sigh.

    I'm betting he has some really really really long strings to deal with.

    Cargo Cult Programming at its finest!






  • Anonymous (unregistered) in reply to Mike R

    What about the fact that the file offset is floating point? Just in case we got half way through a byte?

  • (cs) in reply to Scott Pedersen
    Anonymous:
    *(char*)*va_arg( marker, char * );
    is needed to compensate for 
    va_start( marker, first );

    The malformed va_start means the first va_arg will return the char* format.  The line should read:
    va_start( marker, format);

    As to why he is casting and dereferencing it... well... you're on your own.

    And, of course, the amusing thing is that the whole 'skip the char *' bit isn't actually portable. va_args isn't for reading arguments that the compiler knows are there, and if I'm not mistake, its result is undefined if you do. For all you know, the compiler could be passing all the arguments it knows the exact signatures for in registers.

  • Ellery (unregistered)

    I wonder how he calls this function. What is his value to "first"?

    And I think I know why he needs the -1 for fseek, he probably passed "%d " for the format (there is a space after the %d)

    Also I just realized where is string_write_to pointing to? Is it changing the value of "fomat".  Does he have enough space?

    The strcpy at th end the same thing as the line right before it.

    oh I think almost every line is bad.  Now my head hurts. ;)

  • Morgo (unregistered) in reply to Garcimore
    Anonymous:
    He also had a design pattern of its own :


    if(!condition)
    {
    }
    else
    {
    statements;
    [...];
    }

    I think I know why that is. Imagine this:

    if (a==b) { // Do stuff... } else { // Other stuff... } after compiling (in no particular assembly language):

    cmp a,b bne else // Do stuff... bra after else: // Other stuff... after: ....

    The 'if' path has one extra instruction; the branch to skip over the 'else' code. Looks like your guy heard that 'else' paths always run quicker so decided to use them all the time...

  • (cs) in reply to A. Nonymous
    Anonymous:
    Bah! This is a hoax.

    I agree...
    *(char*)*va_arg( marker, char * );
    This line would crash every time.
    Note that the va_arg is dereferenced immediately - which is ok because the parameter is a pointer.  The result of that is the value of the first literal "char" (0->255) in the string parameter.  This is then cast to a pointer again (char*  at the front) and then dereferenced again!  Since we know that the value will be 0->255 the memory address it attempts to read will be in this range.  Most 32-bit CPUs reserve the memory address range 0->65535 for 16-bit thunks and thus will invoke an invalid memory address exception every single time.

    The only possibility is if the compiler was optimising the function to not-have the final dereference (or cast - very probable since the operation is worthless).  This means that the code would not work in DEBUG mode.  Either the programmer did everything in equivalent RELEASE mode (full optimisation switched on) or it's fake.

  • (cs)

    Jesus...that just made my day after the code review I just went through.  It wouldn't suprise me if he was getting a 250.00/hour IBM salary for that nonsense.  Code like that helps me keep some perspective.

    One could only imagine the code he would have invented if working on the The Flat-File Society XML

  • Anon Coward (unregistered) in reply to Xarium

    I actually tested this to see what would happen. Using a version of gcc, there was no crash.

    It seems as though va_arg is expanded into a __builtin_va_arg function, and if you don't use the result of the function, the dereferencing is optimized away. This happened even with -g -O0 options passed to gcc.

    When I tried to assign the result to a variable, then there was indeed a bus error every time, as expected.

    I must say, once you get beyond the fact that this function's existence in the first place is a major WTF, almost every single line of the implementation is an additional WTF in itself. Impressive... nearly a perfect score I think.

    One fun one that I haven't seen mentioned is that the file is closed and opened a second time (or third if you count the initial write), rather than just using fseek() to move back to the start of the file. I guess his experimentations never led to the discovery of SEEK_SET (let alone stat(), which would be a more efficient way to discover the file length).

    I believe this is real... someone trying to fake a WTF wouldn't dare make it this bad.

  • Sotek (unregistered)

    As absurdly ugly as this all is, I think I at least see why the {}'s are there.

    I bet they tricked an older compiler version into allowing variable declarations, while in c - because you can define in new blocks, which is, of course, what those are.

    Of course, that doesn't explain why they don't preceed ALL variable definitions ... but ...


    ... AAAAAAAAAIIIIEEEEEEEEEEEEEE!

  • (cs) in reply to Gn
    Anonymous:
    Anonymous:
    I particularly liked his filehandle naming convention:

    FILE1 FILE11 FILE111



    He's using base 1 obviously!


    der, its roman numerals. he used 1 coz if he used I it would get lost as part of the word FILE. (FILEI/FILE1) << SEE!!

  • (cs) in reply to RevMike
    RevMike:

    So tell me, what does vector1 * vector2 evaluate to?


    Question: What does equals() evaluate to in some class I've just written?
    Answer: Whatever I want it to. Like, the opposite of what you'd expect.
    Question: What prevents me from using equals() that way?
    Answer: Only common sense. Certainly not the programming language.

    Did I make myself clear, for once? Stop bashing operator overloading.

  • (cs) in reply to Gn

    Anonymous:
    Anonymous:
    I particularly liked his filehandle naming convention: FILE1 FILE11 FILE111


    He's using base 1 obviously!

    Hopefully you mean base 2? Base 1 would consist of only 0, seeing as base 10 consists of 0 thru 9 inclusive, but not 10.

    Drak

  • tellme (unregistered) in reply to RevMike
    RevMike:

    So tell me, what does vector1 * vector2 evaluate to?


    well, you had your chanches with ada, but thrown it away!
    there, if you do

    double u=v1*v2; // you clearly want a scalar product
    vector u=v1*v2; // you clearly want a cross product

    (sorry, it has to be := )

    but no, ansi said that dumb programmer can't handle the overload of the return value......

  • WTFDailyReader (unregistered) in reply to tellme

    You see - this WTF I don't believe. It must be manufactured - look at the C++ code used. There's some pretty advanced constructs used and I hardly believe that someone who manage to use such intricate code doesn't know sprintf! This, IMHO, is a WTF-wannabe!

  • (cs) in reply to Sotek
    Anonymous:
    As absurdly ugly as this all is, I think I at least see why the {}'s are there.

    I bet they tricked an older compiler version into allowing variable declarations, while in c - because you can define in new blocks, which is, of course, what those are.

    No they're not. They open a new block, then immediately close it again. Those {}'s are nothing but an attempt to be cute.
  • (cs) in reply to RevMike

    Depends on what convention you choose.
    I usually use the * operator for the dot product, and ^ for the cross product.

  • dodo (unregistered)

    This reminds me of a piece of code I once used in a project:
    Originally this was taken from the unix vasprintf code, but /dev/null has been replaced with some temp file :]


          9     static inline int vasprintf(char **strp, const char *fmt, va_list ap)
         10     {
         11         FILE *dev_null;
         12         int arg_len;
         13
         14     dev_null    = fopen("C:\devnull.txt", "w");
         15     arg_len     = vfprintf(dev_null, fmt, ap);
         16     if (arg_len != -1) {
         17         *strp   = (char *)calloc((size_t)arg_len + 1, sizeof(char));
         18         arg_len = vsprintf(*strp, fmt, ap);
         19     }
         20     fclose(dev_null);
         21     return arg_len;
         22     }
         23

  • Davey Jones (unregistered) in reply to Alun Jones

    >(can this be used as a random number generator?)

    No, of course not. The docs don't suggest that you can, and seriously; what's the problem for testing that the return value is less than zero?

  • Anonymouse (unregistered) in reply to HAK
    HAK:
    Hooah for reinventing the wheel.  And making it square instead of round, to boot.


    What's much more important is to get the color right. Better form a committee to decide on that...

Leave a comment on “Printf to String”

Log In or post as a guest

Replying to comment #:

Log Out

« Return to Article