patterncMinor

C itoa implementation

Submitted by: @import:stackexchange-codereview·Mar 10, 2026·

Viewed 0 times

implementationitoastackoverflow

Problem

As an exercise, I limited my self to the man pages. I didn't look for a previous existing implementation because I wanted to see how well I could do on my own. I also tried to handle as many cases as I could think of. I am sure there are weaknesses so feel free to point them out. Where can I improve my usage of the C standard library, etc.

char *
itoa(int i)
{
  short digit_cnt;
  short index;
  void *ret;
  char digit;
  int tmp;

  errno = 0;
  feclearexcept(FE_ALL_EXCEPT);
  if (i == 0) {
    digit_cnt = 1;
  } else {
    tmp = (i == INT_MIN) ? (i + 1) : i;
    digit_cnt = floor(log10(abs(tmp))) + 1;
    if (errno || fetestexcept(FE_INVALID | FE_DIVBYZERO | FE_OVERFLOW |
      FE_UNDERFLOW)) {
      return NULL;
    }
  }

  if (i < 0) {
    ++digit_cnt;
  }
  ++digit_cnt; // '\0'

  errno = 0;
  if ((ret = malloc(digit_cnt * sizeof(char))) == NULL || errno) {
    print_error_msg(errno);
    free(ret);
    return NULL;
  }

  // This made debugging easier.
  memset(ret, '0', digit_cnt * sizeof(char));

  index = digit_cnt;
  ((char*)ret)[--index] = '\0';

  tmp = i;
  do {
    digit = (char)((int)'0' + abs(tmp % 10));
    ((char*)ret)[--index] = digit;
    tmp /= 10;
  } while (tmp != 0);

  if (i < 0) {
    ((char*)ret)[--index] = '-';
  }

  return ret;
}

Solution

Well, the code works. It's definitely not as efficient as it could be, though; you're using floating-point math to accomplish something that could be done entirely in integer operations.

You forgot to #include all the headers you need. By my count, that's

#include   // feclearexcept
#include   // INT_MIN
#include   // floor, log10
#include   // abs
#include   // errno
#include   // memset

Plus wherever print_error_msg comes from; that's an undefined symbol in the code you posted. I'm guessing it does something similar to perror(NULL)?

If I were writing itoa, I'd start by assuming that I already had the memory allocation taken care of. Then filling the buffer is as easy as what you wrote:

size_t result_length = ;
char *result = malloc(result_length + 1);
result[result_length] = '\0';
if (number < 0) {
    result[0] = '-';
    number = -number;
}
for (int i = result_length, n = number; n != 0; n /= 10) {
    result[--i] = '0' + (n % 10);
}

(I'd special-case 0 and INT_MIN at the top of the function so that we don't have to worry about them down here.)

Then the only missing piece is the `. But that's easy — we just need to write some code to count the digits of number. And we already have some code to print the digits of number

! So I'd just repeat that code again.

int result_length = (number < 0);
for (int n = number; n != 0; n /= 10) {
    result_length += 1;
}


There — the only headers we needed were

(for malloc) and possibly (for INT_MIN

).

Notice that I wrote

'0' + x where you'd written (char)((int)'0' + x). C isn't Pascal or Python; there's no need to jump through hoops like chr(ord('0') + x)

 here. Characters in C are small integers, and you can do math on them just as you can on any other integer type.

Moreover, you should do math on character types, if it eliminates the need for a cast operation. Type-casting in C and C++ is a code smell; if you really need a cast, something is horribly wrong with your code. In this case, fortunately, you don't need one.

Another place your code uses casts is

void *ret;
...
((char*)ret)[--index] = '\0';


Kudos for using

'\0' instead of 0

; that's a nice way to indicate to the reader what this particular zero semantically represents. But kill the cast! In this case, you think you need the cast because

ret is a void. However, the only places you refer to ret, you cast it to char. This is a huge honking sign that ret really wants to be char*

.

char *ret;
...
ret[--index] = '\0';


Problem solved.

ret had no business being a void*` in the first place.

No comment on the theoretical correctness of the floating-point stuff; because, as I said, I wouldn't have used floating-point to print integers in the first place. I have tested your code on all 4 billion 32-bit integers, though, and can report that it works fine in practice.

Code Snippets

#include <fenv.h>  // feclearexcept
#include <limits.h>  // INT_MIN
#include <math.h>  // floor, log10
#include <stdlib.h>  // abs
#include <errno.h>  // errno
#include <string.h>  // memset

size_t result_length = <some magic>;
char *result = malloc(result_length + 1);
result[result_length] = '\0';
if (number < 0) {
    result[0] = '-';
    number = -number;
}
for (int i = result_length, n = number; n != 0; n /= 10) {
    result[--i] = '0' + (n % 10);
}

int result_length = (number < 0);
for (int n = number; n != 0; n /= 10) {
    result_length += 1;
}

void *ret;
...
((char*)ret)[--index] = '\0';

char *ret;
...
ret[--index] = '\0';

Context

StackExchange Code Review Q#120095, answer score: 9

Revisions (0)

No revisions yet.