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 <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
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 = <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);
}
(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 <some magic>
. 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 <stdlib.h>
(for malloc
) and possibly <limits.h>
(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.