Writing an itoa function

Question

Here is a basic itoa function that I've written trying to use recursion:

void itoa(int number, char buffer[])
{
 static int idx=0, size=0;
 if (number == 0) {
 buffer[size] = '0円';
 idx = size = 0; // reset for next call
 }
 else {
 size++, idx++;
 itoa(number/10, buffer);
 buffer[size-idx--] = (number % 10) + '0';
 }
}

How does it look? It is not thread-safe due to the static, but is that an issue for a function such as this? If so, how could I update that? What other ways could I make improvements to it?

• vnp: How does it look? - scary. Besides, if number is 0 to begin with, it doesn't give the correct result.
• Toby Speight: Why did you want a recursive function? That's usually a poor choice in C if there's a simple iterative version possible.
• Mr R: How do you know the buffer is big enough? The recursion doesn't seem to place things in the right place... Couldn't you just use sprintf?
• CiaPan: @MrR 1. It's up to caller to provide the buffer long enough. 3. I suppose the printf functions family utilizes the same routine for int arguments as itoa() does, so redirecting itoa to sprintf may be a kind of an infinite loop....
• CiaPan: Your code doesn't seem to care about negative values of number...

Answer 1

How does it look?

Not so good.

1. Not thread safe.

2. Does not handle negative numbers well.

3. Forms "" with 0.

4. Prone to buffer overflow.

5. itoa() is not a standard function, yet it commonly returns a pointer.

A test harness

int main(void) {
 char buf[100];
 int test[] = {123, 456, 0, - 42, INT_MAX, INT_MIN};
 int n = sizeof test / sizeof *test;
 for (int i = 0; i < n; i++) {
 itoa(test[i], buf);
 printf("%-11d <%s>\n", test[i], buf);
 }
 return 0;
}

Output

123 <123>
456 <456>
0 <>
-42 <,.>
2147483647 <2147483647>
-2147483648 <./,),(-*,(>

---

It is not thread-safe due to the static, but is that an issue for a function such as this?

Yes. Thread safety is expected.

---

If so, how could I update that? What other ways could I make improvements to it?

I really do not think this is a good place to use recursion given the potential for buffer overflow is a fair complication.

But if one must use recursion, consider adding error checking and test for various sorts of int including 0, INT_MAX, INT_MIN.

static char* itoa_helper(int number, size_t sz, char buffer[]) {
 if (sz == 0) {
 return NULL;
 }
 if (number <= -10) {
 buffer = itoa_helper(number / 10, sz - 1, buffer);
 if (buffer == NULL) {
 return NULL;
 }
 number %= 10;
 }
 *buffer = (char) ('0' - number);
 return buffer + 1;
}
char* itoa_recursive_alt(int number, size_t sz, char buffer[sz]) {
 if (sz == 0 || buffer == NULL) {
 return NULL;
 }
 char *s = buffer;
 if (number >= 0) {
 // Flip pos numbers to neg as neg range is greater.
 number = -number;
 } else {
 sz--;
 if (sz == 0) {
 *buffer = '0円';
 return NULL;
 }
 *s++ = '-';
 }
 s = itoa_helper(number, sz-1, s);
 if (s == NULL) {
 *buffer = '0円';
 return NULL;
 }
 *s = 0;
 return buffer;
}
int main(void) {
 char buf[100];
 int test[] = {123, 456, 0, -42, INT_MAX, INT_MIN};
 int n = sizeof test / sizeof *test;
 for (int i = 0; i < n; i++) {
 // char *s = itoa_recursive_alt(test[i], sizeof buf, buf);
 char *s = itoa_recursive_alt(test[i], sizeof buf, buf);
 if (s == NULL)
 s = "NULL";
 printf("%-11d <%s> <%s>\n", test[i], s, buf);
 }
 return 0;
}

Output

123 <123> <123>
456 <456> <456>
0 <0> <0>
-42 <-42> <-42>
2147483647 <2147483647> <2147483647>
-2147483648 <-2147483648> <-2147483648>

• carl.hiass: awesome, thank you so much for putting in the time. A few questions: (1) what's the downside of not doing the covert-to-negative? (2) what's the downside of not passing a size to the itoa function? Can't the callee determine the size and just set a max-length and return an error if there's truncation?
• chux: @carl.hiass 1) what's the downside of not doing the covert-to-negative? --> Explain how you are going to handle INT_MIN which is less than -INT_MAX. 2) what's the downside of not passing a size to the itoa function? --> Risking buffer overflow 3) Can't the callee determine the size and just set a max-length and return an error if there's truncation? --->Unless the caller provides a size, the callee does not know how much is too much. The callee does not control the amount of buffer that can be used. The caller knows though.
• chux: @carl.hiass Was recursion a requirement?
• carl.hiass: no I was just learning about it and trying out with that. I thought it might get rid of the need to reverse (which it does) but obviously there are a lot of other issues it introduces.
• carl.hiass: I've written an updated version based on your help -- thanks again! codereview.stackexchange.com/questions/257428/…