• (cs)

    What's up with all the {} embedded in there?  Please tell me that's Alex's new way of showing a snippet...

  • James Carr (unregistered)

    Oh well, at least it was caught!

  • Dan (unregistered) in reply to Whackjack

    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...

  • Bert (unregistered) in reply to James Carr

    I especially love this part:

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

  • (cs)
    Alex Papadimoulis:
    // 2 is to the end ... it took some experamenting
// -1 is so we go back and dont get the blank after it
fseek( FILE11, -1, 2 );


    Best comment in the whole WTF.  Good thing he didn't try to read any documentation, or he might've stumbled upon sprintf.
  • Kooooch (unregistered)

    Finally! Comments that aid in understanding the code!

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

    And, oh yeah, "FIRST!"

  • (cs) in reply to Kooooch

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

  • (cs) 
    // this is required, or it will not work for some reason
    *(char*)*va_arg( marker, char * );
    Holy crap, what is that?! Is is a function call? A declaration? I haven't got much experience with C, but I always carried this delusion that C wasn't too complicated.

  • boohiss (unregistered) in reply to Whackjack

    Seriously. I've never seen "{}" used like this anywhere before. Is it supposed to be a seperator or something? What's wrong with "// *******" or the like? Someone had to teach him that...

  • (cs)

    Alex, may I ask wether you sprinkled all the spelling mistakes in this stuff of if they're genuine on top of the programmatic WTF?

  • Colin (unregistered)

    How does one know about vfprintf but not sprintf?  Like knowing foreach() and not for() or XPath but not getElementById() or unsigned int but not int.....  

  • (cs)

    Clearly this is a new form of 'C', what with the inline function declarations:

    (char)*va_arg( marker, char * );


    the empty blocks {} (obviously a special command to their proprietary compiler) and the lack of a string formatting function.

  • NoName (unregistered)

    Please kill this "programmer". There is nothing more to say.

  • (cs)

    Wow!  So this guy never heard of pipes?  Assuming the code doesn't need to be thread safe, he could have done one mkfifo() call, and use dit over and over.

  • toxik (unregistered)
    Alex Papadimoulis:

    A lot of programmers have lost all hope after seeing this website. After all, most of the code I post here was found out in the wild, on real, live, production systems that some of you actually maintain. But today I'd like to offer a glimmer of hope: there are actually people who are out there who are fighting the good fight, doing their best to makes sure that code like today's never makes it to production.

    You should all be thanking Kirk L. for being one of those guys. He discovered this in a code review of a colleague. The code also explained why there were hundreds of temp files in the working directory came and a huge leakage of file pointers. If you're not well-versed in C++, suffice it to say that while most people would simply use sprintf, our coder felt it better to format strings by writing temporary files to disk and reading them back ... 

    void printf_to_string( int first, char * fomat, ... )
    {
      va_list marker;

      va_start( marker, first );

      // this is required, or it will not work for some reason
      *(char*)*va_arg( marker, char * );

      // string to write to
      // ** MUST BE FIRST AFTER FORMAT IN FUNCTION COMAND LINE **

      {} /* make the variable */
      char * string_write_to = va_arg( marker, char * );
      char temporary_file_name[ 19 ];
      char * temorary_file_name_name ;

      strcpy( temporary_file_name, "thefileXXXX" );

      temorary_file_name_name = _mktemp( temporary_file_name );

      {}

      FILE * FILE1 = fopen( temporary_file_name, "w" );

      vfprintf( FILE1, fomat, marker );

      fclose( FILE1 );

      {} FILE * FILE11 = fopen( temporary_file_name, "r" ); {}

      // 2 is to the end ... it took some experamenting
      // -1 is so we go back and dont get the blank after it
      fseek( FILE11, -1, 2 ); 

      {}
      double bignumberjustincase = ftell( FILE11 );

      fclose( FILE1 );

      FILE * FILE111 = fopen( temporary_file_name, "r" );

      read( _fileno( FILE111 ), string_write_to, (short)bignumberjustincase );

      int stopHere = (long)string_write_to + (int)0;

      stopHere = stopHere + (bignumberjustincase);
      
      if ( stopHere != 0 ) 
      {
        ((char*)stopHere)[ 0 ] = '\0';
        strcpy( string_write_to + int(bignumberjustincase ), "" );  
        va_end( marker );
      }
    }

    Am I the only one noticing that 
    double bignumberjustincase = ftell( FILE11 );

    Or what? BIG NUMBER JUST IN CASE? That is casted to short later on? WTF?! :D

  • James Carr (unregistered) in reply to Kooooch
    Kooooch:

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



    That's why you should avoid screaming "FIRST" when you post... it only makes people look more idiotic by trying to proudly let everyone know they got the first post when they are the 10th reply. ;)

    Of course, anyone who feels special and proud because they have the first post is idiotic enough. This isn't slashdumb... er... slashdot after all. ;)
  • Scott Pedersen (unregistered) in reply to James Carr 
    *(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.
  • Martin (unregistered) 

    I really like this comment:

      // 2 is to the end ... it took some experamenting
      // -1 is so we go back and dont get the blank after it
      fseek( FILE11, -1, 2 ); 

    Somebody please sent this guy a manpage or a link to google. 
    Typing fseek and hitting
    "I feel lucky" in google would have given him the name(and value but he don't need that) 
    of the argument
    to seek from the end.
     

  • (cs) in reply to Brendan Kidwell

    This look like an awful workaround that I don't want to try and understand to the fact that he apparently couldn't understand the docs for stdargs.
    I actually think he read the msdn docs, because their samples use the same names for the variables: first and marker.

    Here is a snippet from the beginning of an example:



    int average( int first, ... )
    {
       int count = 0, sum = 0, i = first;
       va_list marker;

        va_start( marker, first );     /* Initialize variable arguments. */

  • (cs) in reply to James Carr

    temporary_file_name and temporary_file_name_name..................classic

  • (cs) in reply to Zlodo

    (...second attempt, posting with this forum software is less enjoyable that eating broken glass)

    This look like an awful workaround that I don't want to try and understand to the fact that he apparently couldn't understand the docs for stdargs.
    I actually think he read the msdn docs, because their samples use the same names for the variables: first and marker.

    Here is a snippet from the beginning of an example:


    int average( int first, ... )
    {
       int count = 0, sum = 0, i = first;
       va_list marker;

        va_start( marker, first );     /* Initialize variable arguments. /

    (snip)


    So he apparently didn't get that the second argumment to va_start is the last argument before the ellipsis, regardless of its type.
    It's amazing that he found about vfprintf, which is rarely used, and entirely miss sprintf.

    Other WTF:
    The usage of read with a FILE     which require using _fileno, instead of just using fread which takes a FILE* directly.
    The usage of the antiquated and dangerous C formatting functions and file API instead of C?? iostream/stringstream

  • (cs)

    What i find incredible is that he knows/found out how to use va_list but doesnt know / bother to look up spintf / snprintf ( snprinf may be usefull if the input length isnt limited ).
    Also he asumes that he was the first to think of it and thus has to impletement it himself.

    // 2 is to the end ... it took some experamenting
    // -1 is so we go back and dont get the blank after it
    fseek( FILE11, -1, 2 );
    I guess SEEK_END must be equal to 2 on this machine .

    strcpy( temporary_file_name, "thefileXXXX" );
    Gee that is descritive. Maybe 'theStudpidFileXXXXXX' would be better.

    double bignumberjustincase = ftell( FILE11 );
    Using a floating point to store a filelength ......
    The varible naming is .... well ick.

     read( _fileno( FILE111 ), string_write_to, (short)bignumberjustincase );
    And then cast it to a short


  • (cs) in reply to Bert
    Anonymous:
    I especially love this part:

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


    Yay for cargo cult programming!
  • Clinton Pierce (unregistered)

    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?

  • (cs) in reply to Clinton Pierce
    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?


    snprintf.

    int snprintf(char *str, size_t size, const char *format, ...);

  • Chris (unregistered) in reply to ItsAllGeekToMe

    I particularly liked his filehandle naming convention:

    FILE1 FILE11 FILE111

  • (cs) in reply to NoName

    <font size="2">@If(@current_floor() > 4; @shove_out_window();
           @wall_composed_of(BRICK) || @wall_composed_of(CONCRETE); @bash_head_in();
           @promote_to_management())

    </font>

  • (cs) in reply to Clinton Pierce

    Since he declares variables in between statements, this is C++.

    So he could have done:
    stringstream sstr;
    sstr << "blabla " << somevariable << someotherone etc.

    The destination buffer is dynamically allocated so you can't overflow, it is disposed of automatically as soon as you leave the scope where sstr is defined, and you can't have mismatches as you can with C formatting functions between formatting characters and the actual datatype.

  • (cs) 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...

     

    comments like this help me get through my day. Keep up the good sarcasm!!!

  • (cs) in reply to Brendan Kidwell
    Brendan Kidwell:
    // this is required, or it will not work for some reason
    *(char*)*va_arg( marker, char * );
    Holy crap, what is that?! Is is a function call? A declaration? I haven't got much experience with C, but I always carried this delusion that C wasn't too complicated.



    C does something pretty clever.  Many popular languages put function parameters on the stack from left to right.  C (and C++) reverse this calling convention, putting function parameters on the stack form right to left.

    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.

    The call to va_start initializes a control structure that allows the stack to be popped.  Then each call to va_args pops each subsequent value off the stack, the type of the value is given by the type parameter.  (va_args here is not a function but a macro, that 'char *' parameter will get wrapped in a sizeof in order to pop the correct number of bytes.)  va_end closes up all the processing.

    The problem is that va_start is normally initialized with the name of the last named parameter.  In this case, it was called incorrectly, so the developer popped an extra pointer off the stack in order to get rid of the extra named parameter before getting the unnamed parameters.

  • (cs) in reply to Zlodo
    Zlodo:
    Since he declares variables in between statements, this is C++.

    So he could have done:
    stringstream sstr;
    sstr << "blabla " << somevariable << someotherone etc.

    The destination buffer is dynamically allocated so you can't overflow, it is disposed of automatically as soon as you leave the scope where sstr is defined, and you can't have mismatches as you can with C formatting functions between formatting characters and the actual datatype.


    But then he'd be using shift operators on these things, a WTF in itself.  Oh, operator overloading?  That makes everything much more readable.

    :)
  • (cs)

    Hooah for reinventing the wheel.  And making it square instead of round, to boot.

  • (cs) in reply to Clinton Pierce
    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?



    No, the technique allocates at least an entire block of disk, generally only allocatable in quanta of 512 or some multiple of 512.  (differs machine to machine).

  • (cs)

    "temorary_file_name_name", "bignumberjustincase" and "FILE111"...

    Classic.

  • (cs) in reply to RevMike
    But then he'd be using shift operators on these things, a WTF in itself.

    It's not the shift operator here, it quite obviously the streaming operator.
    Yeah, its definition of it as a streaming operator isn't hardcoded in the compiler like it's definition as the shifting operator for numeric datatypes, but this is a very specious argument.

    Oh, operator overloading?  That makes everything much more readable.


    I totally agree.
    vector = vector1 + vector2 * scale
    is barely understandable compared to something like
    vector = vector1.add( vector2.multiply( scale ) ).
  • Alun Jones (unregistered) in reply to paranoidgeek

    paranoidgeek:
    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?

    snprintf.

    int snprintf(char *str, size_t size, const char *format, ...);

    Uh... no.

    snprintf allows you to avoid overflowing your allocation, by telling the function how much you allocated, yes.  What it does not do, that "Anonymous" was trying to suggest, is actually tell you how many bytes the sprintf operation will be creating.

    There's a difference between calculating the allocated size before allocating it, and allocating first, then detecting an overflow afterwards.

  • (cs)

    Now, see, I would have implemented this one a little bit differently. If you're using bignumberjustincase, why not go for:

    largeintegerifweneedit

    humongousvaluecauseyouneverknow

    gargantuandoubletoprotectusfromunexpectedsitutationskindalikeyouknowright

    [H]

  • Morbii (unregistered)

    I love how he opens the file with several different handles and only closes one of them (twice, to boot!)

    As mentioned, sprintf can be bad if you don't know what you're dealing with length-wise.  I'd love to see him try to figure out snprintf, though (especially if the string ended without a NULL terminator!)

  • (cs) in reply to Zlodo
    Zlodo:
    But then he'd be using shift operators on these things, a WTF in itself.

    It's not the shift operator here, it quite obviously the streaming operator.
    Yeah, its definition of it as a streaming operator isn't hardcoded in the compiler like it's definition as the shifting operator for numeric datatypes, but this is a very specious argument.

    Oh, operator overloading?  That makes everything much more readable.


    I totally agree.
    vector = vector1 + vector2 * scale
    is barely understandable compared to something like
    vector = vector1.add( vector2.multiply( scale ) ).


    So tell me, what does vector1 * vector2 evaluate to?
  • a0a (unregistered)

    I love his way of pragmatic programming.. you can tell he just KNOWS what the code is doing properly.  After all, he probably fed all his 31337 values into it..

    You made my day again, Alex.


  • Anomalous Cowherd (unregistered) in reply to Alun Jones
    Anonymous:

    snprintf allows you to avoid overflowing your allocation, by telling the function how much you allocated, yes.  What it does not do, that "Anonymous" was trying to suggest, is actually tell you how many bytes the sprintf operation will be creating.

    There's a difference between calculating the allocated size before allocating it, and allocating first, then detecting an overflow afterwards.



    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.
  • (cs)

    I still don't understand how someone who knows how to use the variable argument list doesn't know about sprintf... This dude had to be on crack. That is the only logical explanation. There is just so much wrong with this code that it hurts me to look at it. To actually think that this person has/had a job is astounding. Are college professors passing people in programming courses just because they have to? Are they using grading curves? If no one in the class can code, then no one should pass. Grading coding and math clases on scales is what produces code like this. No good can come of it!

    bignumberjustincase has got to be THE absolute WORST name for ANY variable EVER... EVER!

  • (cs) in reply to GoatCheez
    GoatCheez:
    bignumberjustincase has got to be THE absolute WORST name for ANY variable EVER... EVER!


    It could have been in Hungarian notation.
  • Anonymous Coward (unregistered) in reply to Zlodo

    http://www.geocities.com/nodotus/hbglass.html

  • (cs) in reply to toxik
    Anonymous:
    Alex Papadimoulis:

    A lot of programmers have lost all hope after seeing this website. After all, most of the code I post here was found out in the wild, on real, live, production systems that some of you actually maintain. But today I'd like to offer a glimmer of hope: there are actually people who are out there who are fighting the good fight, doing their best to makes sure that code like today's never makes it to production.

    You should all be thanking Kirk L. for being one of those guys. He discovered this in a code review of a colleague. The code also explained why there were hundreds of temp files in the working directory came and a huge leakage of file pointers. If you're not well-versed in C++, suffice it to say that while most people would simply use sprintf, our coder felt it better to format strings by writing temporary files to disk and reading them back ... 

    void printf_to_string( int first, char * fomat, ... )
    {
      va_list marker;

      va_start( marker, first );

      // this is required, or it will not work for some reason
      *(char*)*va_arg( marker, char * );

      // string to write to
      // ** MUST BE FIRST AFTER FORMAT IN FUNCTION COMAND LINE **

      {} /* make the variable */
      char * string_write_to = va_arg( marker, char * );
      char temporary_file_name[ 19 ];
      char * temorary_file_name_name ;

      strcpy( temporary_file_name, "thefileXXXX" );

      temorary_file_name_name = _mktemp( temporary_file_name );

      {}

      FILE * FILE1 = fopen( temporary_file_name, "w" );

      vfprintf( FILE1, fomat, marker );

      fclose( FILE1 );

      {} FILE * FILE11 = fopen( temporary_file_name, "r" ); {}

      // 2 is to the end ... it took some experamenting
      // -1 is so we go back and dont get the blank after it
      fseek( FILE11, -1, 2 ); 

      {}
      double bignumberjustincase = ftell( FILE11 );

      fclose( FILE1 );

      FILE * FILE111 = fopen( temporary_file_name, "r" );

      read( _fileno( FILE111 ), string_write_to, (short)bignumberjustincase );

      int stopHere = (long)string_write_to + (int)0;

      stopHere = stopHere + (bignumberjustincase);
      
      if ( stopHere != 0 ) 
      {
        ((char*)stopHere)[ 0 ] = '\0';
        strcpy( string_write_to + int(bignumberjustincase ), "" );  
        va_end( marker );
      }
    }

    Am I the only one noticing that 
    double bignumberjustincase = ftell( FILE11 );

    Or what? BIG NUMBER JUST IN CASE? That is casted to short later on? WTF?! :D

    Yeah, I saw it. It barely registered amidst the sea of wtf

  • Magnus H (unregistered) in reply to RevMike
    RevMike:

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

    Hopefully the scalar product of vector1 and vector2 (so that * means · as with numbers). The cross product has to use some other symbol.
  • Alun Jones (unregistered) in reply to Anomalous Cowherd

    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?)

  • Suck My Lisp (unregistered) in reply to RevMike
    RevMike:
    So tell me, what does vector1 * vector2 evaluate to?

    A double; the dot product of the two vectors.  We'll use "<<" for "multiply by the transpose," (we'll return an array), ">>"  for "pairwise multiply" (we'll return a vector) then we'll drink a beer and laugh at the poor saps who'll be maintaining our code when we move on.
  • (cs) in reply to Alun Jones

    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?

  • (cs)

    this is freaking painful to even look at...

Leave a comment on “Printf to String”

Log In or post as a guest

Replying to comment #:

Log Out

« Return to Article