What is the proper way of implementing a good "itoa()" function?
The only actual error is that you don't check the return value of malloc
for null.
The name itoa
is kind of already taken for a function that's non-standard, but not that uncommon. It doesn't allocate memory, rather it writes to a buffer provided by the caller:
char *itoa(int value, char * str, int base);
If you don't want to rely on your platform having that, I would still advise following the pattern. String-handling functions which return newly allocated memory in C are generally more trouble than they're worth in the long run, because most of the time you end up doing further manipulation, and so you have to free lots of intermediate results. For example, compare:
void delete_temp_files() {
char filename[20];
strcpy(filename, "tmp_");
char *endptr = filename + strlen(filename);
for (int i = 0; i < 10; ++i) {
itoa(endptr, i, 10); // itoa doesn't allocate memory
unlink(filename);
}
}
vs.
void delete_temp_files() {
char filename[20];
strcpy(filename, "tmp_");
char *endptr = filename + strlen(filename);
for (int i = 0; i < 10; ++i) {
char *number = itoa(i, 10); // itoa allocates memory
strcpy(endptr, number);
free(number);
unlink(filename);
}
}
If you had reason to be especially concerned about performance (for instance if you're implementing a stdlib-style library including itoa
), or if you were implementing bases that sprintf
doesn't support, then you might consider not calling sprintf
. But if you want a base 10 string, then your first instinct was right. There's absolutely nothing "incorrect" about the %d
format specifier.
Here's a possible implementation of itoa
, for base 10 only:
char *itobase10(char *buf, int value) {
sprintf(buf, "%d", value);
return buf;
}
Here's one which incorporates the snprintf-style approach to buffer lengths:
int itobase10n(char *buf, size_t sz, int value) {
return snprintf(buf, sz, "%d", value);
}
// Yet, another good itoa implementation
// returns: the length of the number string
int itoa(int value, char *sp, int radix)
{
char tmp[16];// be careful with the length of the buffer
char *tp = tmp;
int i;
unsigned v;
int sign = (radix == 10 && value < 0);
if (sign)
v = -value;
else
v = (unsigned)value;
while (v || tp == tmp)
{
i = v % radix;
v /= radix;
if (i < 10)
*tp++ = i+'0';
else
*tp++ = i + 'a' - 10;
}
int len = tp - tmp;
if (sign)
{
*sp++ = '-';
len++;
}
while (tp > tmp)
*sp++ = *--tp;
return len;
}
// Usage Example:
char int_str[15]; // be careful with the length of the buffer
int n = 56789;
int len = itoa(n,int_str,10);
I think you are allocating perhaps too much memory. malloc(8*sizeof(int))
will give you 32 bytes on most machines, which is probably excessive for a text representation of an int.
A good int
to string or itoa()
has these properties;
- Works for all
[INT_MIN...INT_MAX]
, base[2...36]
without buffer overflow. - Does not assume
int
size. - Does not require 2's complement.
- Does not require
unsigned
to have a greater positive range thanint
. In other words, does not useunsigned
. - Allows use of
'-'
for negative numbers, even whenbase != 10
.
Tailor the error handling as needed. (needs C99 or later):
char* itostr(char *dest, size_t size, int a, int base) {
// Max text needs occur with itostr(dest, size, INT_MIN, 2)
char buffer[sizeof a * CHAR_BIT + 1 + 1];
static const char digits[36] = "0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ";
if (base < 2 || base > 36) {
fprintf(stderr, "Invalid base");
return NULL;
}
// Start filling from the end
char* p = &buffer[sizeof buffer - 1];
*p = '\0';
// Work with negative `int`
int an = a < 0 ? a : -a;
do {
*(--p) = digits[-(an % base)];
an /= base;
} while (an);
if (a < 0) {
*(--p) = '-';
}
size_t size_used = &buffer[sizeof(buffer)] - p;
if (size_used > size) {
fprintf(stderr, "Scant buffer %zu > %zu", size_used , size);
return NULL;
}
return memcpy(dest, p, size_used);
}