# Converting decimal integers to a string representation in an arbitrary base between 2 and 26

This code takes an integer and returns a string representing the value in a different base. The value for the base can range between 2 and 26. I have had someone already look over this code and they told me that I was committing a "cardinal sin" of memory management and referred me to the documentation for malloc(). I am confused as to what he is referring to. The only thing I have come up with so far is that I do not check to see if memory allocation succeeds when using calloc() or malloc(). Are there any other things that could or should be improved?

char int_to_char(int number){
if (number > 9) return (char)(((int)'A') + number - 10);
else return (char)(((int)'0') + number);
}

int change_base(char* output, int buffer_size, int decimal_number, int base){
//check for valid parameters
if((base < 2) || (base > 26)) return -1;        //range error

int output_i = 0;
int tmp_string_i = 0;
int dividend;
char remainder;
char * tmp_string = calloc(buffer_size, sizeof(char));
memset(output, '\0', buffer_size*sizeof(char));

//check for negative input
if(decimal_number < 0){
output[0] = '-';
dividend = -decimal_number;
output_i++;
}
else dividend = decimal_number;

//find digits
while(dividend / base != 0){
remainder = int_to_char(dividend % base);
dividend = dividend / base;
tmp_string[tmp_string_i] = remainder;
tmp_string_i++;
if(tmp_string_i + 1 > buffer_size){     //+1 for the extra negative sign
free(tmp_string);
return -2;      //buffer size error
}
}
remainder = int_to_char(dividend);
tmp_string[tmp_string_i] = remainder;

//copy tmp_string to output in reverse order
for(; tmp_string_i >= 0; tmp_string_i--){
output[output_i] = tmp_string[tmp_string_i];
output_i++;
}
free(tmp_string);
return 0;
}


I have also used this for testing:

int main(){
char * output = calloc(100, sizeof(char));
int ret_val = 0;

//test cases
ret_val = change_base(output, 100, 37, 8);
printf("%s\t%d\n", output, ret_val);
ret_val = change_base(output, 100, -45, 8);
printf("%s\t%d\n", output, ret_val);
ret_val = change_base(output, 100, 33945877, 8);
printf("%s\t%d\n", output, ret_val);
ret_val = change_base(output, 100, 32, -7);
printf("%s\t%d\n", output, ret_val);
ret_val = change_base(output, 100, 37, 4);
printf("%s\t%d\n", output, ret_val);
ret_val = change_base(output, 100, 37, 23);
printf("%s\t%d\n", output, ret_val);
ret_val = change_base(output, 100, 37, 2);
printf("%s\t%d\n", output, ret_val);
ret_val = change_base(output, 100, -375212123, 26);
printf("%s\t%d\n", output, ret_val);
ret_val = change_base(output, 5, -375212123, 26);
printf("%s\t%d\n", output, ret_val);
char * output2 = calloc(5, sizeof(char));
ret_val = change_base(output2, 5, 37, 2);
printf("%s\t%d\n", output2, ret_val);

free(output);
free(output2);
return 0;
}

• dividend = -decimal_number; certainly has trouble when decimal_number == INT_MIN; Nov 1 '14 at 3:14

• The cardinal sin

An assignment tmp_string[tmp_string_i] = remainder; may write beyond an allocated memory. Consider a case of buffer_size being equal to 0. You must test tmp_string_i before the assignment. I don't know what exactly someone had in mind, but you are definitely committing one.

• Unnecessary variables

decimal_number is passed by value; you can mutilate it as much as you want. There is no need to have dividend.

Reversal can be done in-place. There is no need for tmp_string at all. tmp_string_i shall also be gone.

• Finding digits

Executing division dividend / base twice per iteration looks wrong; special-case of the last digit looks also wrong. Using do-while solves both wrongs:

    do {
output[output_i++] = int_to_char(dividend % base);
number = number / base;
} while (number != 0);

• No raw loops

Most loops represent an important algorithm, which deserves to be factored out in a function of its own, for further reuse and for giving it a name. In your case, the loop under the //copy tmp_string to output in reverse order comment implements an algorithm known as reverse.

All that said, this is

• Misc

• The returned string should be terminated by '\0', or its actual length be returned.

• The code happily converts numbers to a base up to 36 (10 digits and 26 letters). I see no reason to limit a base to 26.

• Return codes 0,-1,-2 should be defined as named constants.

• Size variables (e.g. buffer_size) should be unsigned, or better yet of type size_t.

• The decimal_number parameter is not decimal. It is just number. Need to be renamed accordingly.