itoa base function in C

Question

This is a function that converts an integer value to a null-terminated string using the specified base and stores the result in a char array that I must allocate (with malloc). I have decided not to use malloc. Allowed functions: malloc. Loops: while Please review my code.

char *itoa_base(int value, int base)
{
 static char rep[] = "0123456789abcdef";
 static char buf[50];
 char *ptr;
 int num;
 ptr = &buf[49];
 *ptr = '0円';
 num = value;
 if (value < 0 && base == 10)
 value *= -1;
 if (value == 0)
 *--ptr = rep[value % base];
 while (value != 0)
 {
 *--ptr = rep[value % base];
 value /= base;
 }
 if (num < 0 && base == 10)
 *--ptr = '-';
 return (ptr);

• albttx: Be careful, you're close to 25 lines ;) #42

Answer 1

Input checking

1. If the caller passes in a base greater than 17, you will read past the end of rep.
2. If the caller passes in a base of 0, you will divide by zero.
3. If the caller passes in a base of 1, your code will infinite loop.
4. If the caller passes in a negative value but the base is not 10, you will read past the front of rep because your modulus will be negative. You should always make value positive before proceeding with your loop.

Insufficient buffer size

On a system with 64-bit ints and a base of 2, you will need a buffer of size 65 instead of 50. (Or 66 if you allow negative values for all bases).

Code simplification

This code:

if (value == 0)
 *--ptr = rep[value % base];
while (value != 0)
{
 *--ptr = rep[value % base];
 value /= base;
}

could be simplified to:

do {
 *--ptr = rep[value % base];
 value /= base;
} while (value != 0);

I'm not sure from your question whether you were allowed to use a do loop or not. Also, your question says you are supposed to return an allocated buffer but you intentionally didn't, so I'm not sure how to review that part of it.

• chux: Good catch on the "modulus will be negative."
• Junius L: @JS1 thanks, I'm not allowed to use do while the only loop allowed is while.
• chux: Note: if itoa_base(INT_MIN, 2) is called and code coverts that to a string beginning with '-', then buffer space needed for n-bit int is n+2.
• JS1: @chux Thanks, I edited the answer for that.

Answer 2

1. Undefined behavior when value = INT_MIN on common 2's complement machines due to int overflow.

   if (value < 0 && base == 10)
    value *= -1;
   

2. A static result is only good for one buffer that is over written with subsequent calls. The following will certainly fail.

   printf("%s %s\n", itoa_base(123, 12), itoa_base(456, 12));
   

3. char buf[50]; is sufficient 6 byte integers, yet is excessive for smaller one. Use a right-size buffer.

   #include <limits.h>
   #define INT_STR_SIZE (sizeof(int)*CHAR_BIT + 2)
   char buf[INT_STR_SIZE];
   

4. Note that code has no base limitations.

   static char rep[] = "0123456789abcdef";
   ...
   assert(base >= 2 && base <= 16);
   

5. Qualifying - generation with base 10 is arbitrary. In the end - design decision. I recommend making a companion utoa() for unsigned.

    // if (value < 0 && base == 10)
    if (value < 0)
   

6. No need to use if (value == 0) to catch 0 case, just use do loop.

   // if (value == 0)
   // *--ptr = rep[value % base];
   // while (value != 0) {
   // *--ptr = rep[value % base];
   // value /= base;
   // }
   do {
    *--ptr = "0123456789abcdef"[value % base];
    value /= base;
   } while (value != 0);
   

7. Code is missing a closing }

See How to convert int into char* in C (without sprintf) for details to cope with the above issues. (That is a fixed base 10 answer, yet easy enough to modify.)

---

[6/21 Edit]
For fun, below is a recursive solution that solves the above issues. Of course a simple loop could be use instead.

include <limits.h>
#define INT_STR_SIZE (sizeof(int)*CHAR_BIT + 2)
static char *my_itoa_base_helper(char *dest, size_t size, int x, int base) {
 if (size == 0) return NULL;
 int digit = -(x%base);
 x /= base;
 if (x) {
 dest = my_itoa_base_helper(dest, size-1, x, base);
 if (dest == NULL) return NULL;
 }
 *dest = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ"[digit];
 return dest + 1;
}
char *my_itoa_base(char *dest, size_t size, int x, int base) {
 if (base < 2 || base > 36 || size < 2) return NULL;
 char *p = dest;
 if (x < 0) {
 *p++ = '-';
 size--;
 } else {
 x = -x;
 }
 p = my_itoa_base_helper(p, size-1, x, base);
 if (p == NULL) return NULL;
 *p = '0円';
 return dest;
}
// compound literal C99 or later
#define MY_ITOA_BASE(x,base) my_itoa_base((char [INT_STR_SIZE]){""}, INT_STR_SIZE,(x),(base))

Test code & Output

void testi(int x, int base) {
 printf("%12d: base 10 = %11s, base %2d = %s\n", 
 x, MY_ITOA_BASE(x, 10), base, MY_ITOA_BASE(x, base)); 
 }
#include <stdio.h.h>
int main() {
 testi(123, 16);
 testi(-123, 16);
 testi(INT_MIN, 2);
 testi(0, 2);
 testi(INT_MAX, 2);
 testi(INT_MIN, 36);
 return 0;
}
 123: base 10 = 123, base 16 = 7B
 -123: base 10 = -123, base 16 = -7B
 -2147483648: base 10 = -2147483648, base 2 =わ -ひく10000000000000000000000000000000
 0: base 10 = 0, base 2 =わ 0
 2147483647: base 10 = 2147483647, base 2 =わ 1111111111111111111111111111111
 -ひく2147483648: base 10 = -2147483648, base 36 = -ZIK0ZK

• Junius L: thanks for your input, I'm not allowed to use do while.