Itoa gets worse every year



  • A while back, I was asked to look at an issue where numerical data "kept getting
    more and more inaccurate" in newer versions of an application.

    After some searching, I came across some code that converted an integer
    into a string representation of the value.  It used the common itoa
    function, and since it was pretty run-of-the-mill data meant to be interpreted
    by humans, the string was supposed to a base-ten representation.

    No big deal, right?

    Whoever wrote this code must have been studying up on design guidelines
    that favored "avoiding magic numbers," as the code I came across
    looked like this:

    char sTypelib_version[3];
    sTypelib_version[0]=0;
    sTypelib_version[1]=0;
    sTypelib_version[2]=0;
    int typelib_version = 13;
    itoa(typelib_version, sTypelib_version, VERSION_CODE);
    printf("Version %s", sTypelib_version);

    After looking for the value of VERSION_CODE, I finally found it in a global constants
    header file:

    //#define VERSION_CODE 10 //2005
    //#define VERSION_CODE 11 //2006
    //#define VERSION_CODE 12 //2007
    #define VERSION_CODE 13   //2008

    Apparently, this code worked extremely well in 2005 when the tenth version of the application was released.



  • Wow, that's... odd.  I'd say it appears he reversed the radix and value parameters, but both are 13..  O_o

     

    TRWTF is itoa. 



  • @morbiuswilters said:

    Wow, that's... odd.  I'd say it appears he reversed the radix and value parameters, but both are 13..  O_o

     

    TRWTF is itoa. 

    True dat.  sprintf 4 eva!


  • @bstorer said:

    True dat.  sprintf 4 eva!

    sprintf?  More like sprintfail!  It's possible to overflow the buffer if you aren't careful, which is why I always use my own function: 

    char *safe_int2string(long n)
    {
    
        int i = 1;
        char *s;
        long tmp = n;
    
        if (n < 0) {
                i++;
        }
    
        for (;;) {
                i++;
                tmp /= 10;
    
                if (tmp == 0) {
                        break;
                }
        }
    
        s = malloc(i);
    
        if (n < 0) {
                s[0] = '-';
                n *= -1;
        }
    
        i--;
    
        s[i] = '\0';
    
        for (;;) {
                i--;
                s[i] = 48 + (n - ((n / 10) * 10));
                n /= 10;
    
                if (n == 0) {
                        return s;
                }
        }
    

    }



  • @morbiuswilters said:

     

    sprintf?  More like sprintfail!  It's possible to overflow the buffer if you aren't careful, which is why I always use my own function: 

    Yeah, but I take care of that like this:

    char * int2string (int n)
    {
      int size = 1;
      char * s = malloc(size);
      sprintf(s, "%d", n);
      while ((size - 1) < strlen(s))
      {
        free (s);
        size *= 2;
        s = malloc(size);
        sprintf(s, "%d", n);
      }
      return s;
    }
    
    See? Easy.


  • @bstorer said:

    Yeah, but I take care of that like this:

    char * int2string (int n)
    {
      int size = 1;
      char * s = malloc(size);
      sprintf(s, "%d", n);
      while ((size - 1) < strlen(s))
      {
        free (s);
        size *= 2;
        s = malloc(size);
        sprintf(s, "%d", n);
      }
      return s;
    }
    

     

    Nice heap trampling, especially with numbers like -2000000000!



  • @Steven V said:

    Whoever wrote this code must have been studying up on design guidelines
    that favored "avoiding magic numbers,"
    Perhaps instead it should use 'more magic'.



  • @mxsscott said:

    Nice heap trampling, especially with numbers like -2000000000!

    It actually works, but only because malloc will almost always allocate more memory than is requested.  It adds a whole new level to the absurdity of the function.  Unfortunately, bstorer's function doesn't work with strings very well, so I will provide to you guys (free of charge) my "safe cat" function:

     

    char *scat(char *a, char *b)
    {
    
        char *s;
        int sz = 1;
        int i;
    
        for(;;) {
                s = malloc(sz);
                memset(s, 1, sz);
                s[(sz-1)] = '\0';
    
                if (strlen(a)+strlen(b)&lt;strlen(s)) {
                        memset(s, 0, strlen(s)+1);
                        strcat(s, a);
                        strcat(s, b);
                        return s;
                }
    
                for (i=0;i&lt;sz;i++) {
                        sz++;
                        i++; //for some reason we get stuck in loop here unless we inc i again..  maybe another thread changing value of i?
                }
    
                s = realloc(s, sz); // realloc() fast than free() + malloc()
        }
    

    }
     

    Use it wisely, friends.



  • scat! 



  • @morbiuswilters said:

    Unfortunately, bstorer's function doesn't work with strings very well, so I will provide to you guys (free of charge) my "safe cat" function:
    We need to start polluting Google with these functions, and see how long it takes for one to find its way to the front page.



  • @bstorer said:

    We need to start polluting Google with these functions, and see how long it takes for one to find its way to the front page.

    This idea is made of pure win. 



  •  doesn't expertSexchange already do this?


Log in to reply