0

I am trying to recode the itoa function which, given an 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:

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

Which 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 minimum size of the buffer. Using <limits.h> seems to do the trick, but I am not allowed to include headers other than <stdlib.h> and <string.h>. I am also limited to three functions, malloc, free and write. However, I did recode strdup and a lot of functions from the C library.

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?

Improve this question
7
  • Please post an MVCE. Commented Dec 8, 2015 at 22:42
  • 2
    -2147483648 * -1 is overflow in 32bit int. Commented Dec 8, 2015 at 22:43
  • @BLUEPIXY shouldn't the limit be the same on both signs? I don't get that. Commented Dec 8, 2015 at 22:45
  • 3
    32bit int max value is 2147483647. Commented Dec 8, 2015 at 22:46
  • 4
    zero is included as well in the total range of numbers so there is one less number for the positive signed numbers. Commented Dec 8, 2015 at 22:50

2 Answers 2

Reset to default
2

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> #include <string.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. strdup has finally been included in the C Standard as of C23!

Improve this answer
Sign up to request clarification or add additional context in comments.

9 Comments

Corner case: Using unsigned, in general works to cope with INT_MIN. Yet C does not specify that UINT_MAX > INT_MAX. Odd ball platforms use a sign bit for int and have UINT_MAX == INT_MAX and -INT_MIN is not representable as an unsigned.
@chux: true but a very theoretical point these days. All odd ball platforms I know that have UINT_MAX == INT_MAX also use sign/magnitude representation, and therefore -INT_MIN is representable as an unsigned. I'd be interested to see and operate your vintage DS9K for fun ;-).
@chux: I was expecting a more general remark such as: you assume 2's complement. Indeed v = -v avoids integer overflow but might not produce the expected value on (hypothetical) non 2's complement hardware.
In looking over source coded for highly portable, came across the the summing technique on the negative side of int, rather than the positive side. The useful attribute is that since it does not use unsigned at all, that type's properties compared to int are moot. Some find it more difficult to follow, but it does work
How is 2^63 relevant to this?
|
1

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

Improve this answer

4 Comments

@chqrlie Did not notice OP's strrev() with malloc(). Code amended.
@chqrlie My compile pick-up up CHAR_BIT when stdlib.h was included. Agree <limits.h> is the better header file.
Although strdup is not a standard function, it is usually declared in <stdlib.h> so both are needed.
@chqrlie: strdup finally made it to the C Standard! Let's celebrate by including <string.h> which is now specified as the header where it is defined (as allowed as confirmed by the OP's). <limits.h> is not allowed, so a practical limit of (sizeof (int)*8/3 + 3) should be preferred and is an upper bound for all complying architectures.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.