Recoding itoa: error on minimum int

Question

I am trying to recode the itoa function which, given a int, will return a string representing its decimal value. So far, the functions works well:

char        *ft_itoa(int n)
{
    char    s[1024];
    int     i;
    int     neg;

    i = 0;
    neg = 0;
    if (n == 0)
        s[i++] = '0';
    if (n < 0)
    {
        n = n * (-1);
        neg = 1;
    }
    while (n != 0)
    {
        s[i++] = (n % 10) + 48;
        n /= 10;
    }
    if (neg)
        s[i++] = '-';
    s[i] = '\0';
    return (ft_strrev(s));
}

Except for the minimum int value, -2147483648. In that case, the function returns:

"-./,),(-*,("

Wich is... weird. Note that ft_strrev will reverse the result and malloc it. Any clues?

EDIT:

There are quite interesting answers here. I am specially interested in the minium size of the buffer. Using limits.h seems to do the trick, but I am not allowed to include other headers than stdlib.h and string.h. I am also limited in three functions, malloc, free and write. However, I did recode strdup and a lot of functions from libc.

Could someone explain why that line would declare the exact amount of memory I need:

char   buf[sizeof(int) * CHAR_BIT / 3 + 3];

Also,

Using unsigned to compute the digits would avoid the problem with INT_MIN. Bug fix for INT_MIN.

Why?

Answer 1

There are several small issues with your code:

• The buffer is much too large: including sign and null terminator, 24 bytes should suffice. For absolute portability, an upper bound of sizeof(int)*CHAR_BIT/3 + 3 is correct. Not a bug, but wasteful.

• If you store the digits into the buffer from right to left, you don't need the final reverse phase and could call strdup() directly. Simpler and faster.

• Using unsigned to compute the digits would avoid the problem with INT_MIN. Bug fix for INT_MIN.

• Looping for i >= 10 and storing the final digit separately avoids the special case for 0. Simpler and faster, fewer divisions.

• You should use '0' instead of hard coding ASCII value 48. More readable and portable.

Here is a modified version:

#include <limits.h>

char *ft_itoa(int n) {
    char buf[sizeof(int)*CHAR_BIT/3 + 3];
    char *s;
    unsigned int v;

    v = n;
    if (n < 0) {
        v = -v;
    }
    s = buf + sizeof(buf);
    *--s = '\0';
    while (v >= 10) {
        *--s = '0' + v % 10;
        v /= 10;
    }
    *--s = '0' + v;
    if (n < 0)
        *--s = '-';
    return strdup(s);
}

If strdup is not available on your system, it is easy to implement and quite useful if you allocate strings from the heap.

Answer 2

Avoid int overflow (if (n < 0) { n = n * (-1); which caused OP's problem) and embraced the dark (negative) side. Since there are more negative int than positive ones, sum on the negative side.

#include <limits.h>
char *ft_itoa(int n) {
  char s[50];
  // char s[1024];
  int i;
  int neg;

  i = 0;
  neg = 0;
  //if (n == 0)          // special case not need with do loop
  //    s[i++] = '0';

  if (n < 0) {           // Insure n is _not_ positive
    // n = n * (-1);
    neg = 1;
  } else {
    n = -n;              // no overflow possible here
  }
  // while (n != 0)
  do {
    // s[i++] = (n % 10) + 48;
    s[i++] = '0' - (n % 10);   // subtract
    n /= 10;
  } while (n);
  if (neg) s[i++] = '-';
  s[i] = '\0';
  return strdup(strrev(s));
}

For a cleaned-up version

#include <limits.h>

// Compute max size need to represent an `int`
#define INT_DEC_SIZE (sizeof (int)*CHAR_BIT/3 + 3)

char *ft_itoa(int n) {
  char s[INT_DEC_SIZE];
  char *p = &s[sizeof s - 1];
  *p = '\0';
  int i = n;
  if (i > 0) {
    i = -i;
  }
  do {
    p--;
    *p = '0' - (i % 10);
    i /= 10;
  } while (i);
  if (n < 0) *(--p) = '-';
  return strdup(p);
}

INT_DEC_SIZE ref