3
\$\begingroup\$

I have enhanced my itoa implementation, taking the advise from my previous simple itoa implementation in addition to handling multiple radixes and negative numbers other then just base 10. Looking for general feedback.

const static size_t complemnt_digit_count[] = {                                
   0,  0, 32, 20, 16, 14, 12, 12, 11, 10,                                      
  -1,  9,  9,  9,  9,  8,  8,  8,  8,  8,                                      
   8,  8,  7,  7,  7,  7,  7,  7,  7,  7,                                      
   7,  7,  7,  7,  7,  7,  6};                                                 

//-------------------------------------------------------------

size_t                                                                         
charcnta(int i, int base)                                                      
{                                                                              
  int tmp;                                                                     
  short digit_count;                                                           

  if (i < 0 && base != 10) {                                                   
    return complemnt_digit_count[base] * sizeof(char);                         
  }                                                                            

  tmp = i;                                                                     
  digit_count = (i != 0) ? 0 : 1;                                              
  while (tmp) {                                                                
    tmp /= base;                                                               
    ++digit_count;                                                             
  };                                                                           

  if (i < 0) {                                                                 
    ++digit_count;                                                             
  }                                                                            

  return digit_count * sizeof(char);                                           
}   

//-------------------------------------------------------------

char *                                                                         
ip_itoa(int i, char *str, int base)                                            
{                                                                              
  char digit, complement_char;                                                 
  int carry_bit, tmp;                                                          
  size_t index;                                                                

  index = charcnta(i, base);                                                   
  memset(str, '0', sizeof(char) * index);                                      
  str[index] = '\0';                                                           

  tmp = i;                                                                     
  do {                                                                         
    digit = '0' + abs(tmp % base);                                             
    str[--index] = digit;                                                      
    tmp /= base;                                                               
  } while (tmp);                                                               

  if (i < 0 && base != 10) {                                                   
    str = ncomp(str, base);                                                    
  } else if (i < 0) {                                                          
    str[--index] = '-';                                                        
  }                                                                            

  return str;                                                                  
}  

//-------------------------------------------------------------

char *                                                                         
ncomp(char *i, int base)                                                       
{                                                                              
  char curr_char;                                                              
  int max_char_index, complement_index, curr_index, carry, index;              
  size_t len;                                                                  

  const char *digits = "0123456789abcdefghijklmnopqrstuvwxyz";                 

  max_char_index = base - 1;                                                   

  len = strlen(i);                                                             

  // To prevent modifying invalid data do a check first.                       
  for (index = 0; index < len; ++index) {                                      
    if (i[index] >= '0' && i[index] <= '9') {                                  
      curr_index = i[index] - '0';                                             
    } else if (i[index] >= 'a' && i[index] <= 'z') {                           
      curr_index = 10 + i[index] - 'a';                                        
    } else {                                                                   
      return NULL;                                                             
    }                                                                          

    if (curr_index > max_char_index) {                                         
      return NULL;                                                             
    }                                                                          
  }                                                                            

  // Calculate the (n - 1)'s complement.                                       
  for (index = 0; index < len; ++index) {                                      
    if (i[index] >= '0' && i[index] <= '9') {                                  
      curr_index = i[index] - '0';                                             
    } else if (i[index] >= 'a' && i[index] <= 'z') {                           
      curr_index = 10 + i[index] - 'a';                                        
    }                                                                          

    complement_index = max_char_index - curr_index;                            
    char curr_char = digits[complement_index];                                 
    i[index] = curr_char;                                                      
  }                                                                            

  // Now add 1, note we drop the carry bit on the most significant digit.      
  carry = 1; // We start at 1 because we want to add one initially.            
  for (index = len - 1; index >= 0; --index) {                                 
    if (i[index] >= '0' && i[index] <= '9') {                                  
      curr_index = i[index] - '0';                                             
    } else if (i[index] >= 'a' && i[index] <= 'z') {                           
      curr_index = 10 + i[index] - 'a';                                        
    }                                                                          

    if (curr_index + carry >= base) { // We have to carry
      i[index] = '0';                                                          
      carry = 1;                                                               
    } else {                                                                   
      i[index] = digits[curr_index + carry];                                   
      carry = 0;                                                               
    }                                                                          
  }                                                                        

  return i;                                                                    
}

For reference here are some unit tests:

  base = 2;                                                                    
  {                                                                            
    {                                                                          
      test_val = INT_MIN;                                                      
      test_val_str = ip_itoa(test_val, buff, base);                            
      ck_assert_str_eq("10000000000000000000000000000000", test_val_str);      
      test_val_str = NULL;                                                     
    }                                                                          

    {                                                                          
      test_val = INT_MAX;                                                      
      test_val_str = ip_itoa(test_val, buff, base);                            
      ck_assert_str_eq("1111111111111111111111111111111", test_val_str);       
      test_val_str = NULL;                                                     
    }                                                                          

    {                                                                          
      test_val = -1;                                                           
      test_val_str = ip_itoa(test_val, buff, base);                            
      ck_assert_str_eq("11111111111111111111111111111111", test_val_str);      
      test_val_str = NULL;                                                     
    }                                                                          

    {                                                                          
      test_val = 1;                                                            
      test_val_str = ip_itoa(test_val, buff, base);                            
      ck_assert_str_eq("1", test_val_str);                                     
      test_val_str = NULL;                                                     
    }                                                                          

    {                                                                          
      test_val = -1024;                                                        
      test_val_str = ip_itoa(test_val, buff, base);                            
      ck_assert_str_eq("11111111111111111111110000000000", test_val_str);      
      test_val_str = NULL;                                                     
    }                                                                          
  } 

  base = 10;                                                                   
  {                                                                            
    {                                                                          
      test_val = 0;                                                            
      test_val_str = ip_itoa(test_val, buff, base);                            
      ck_assert_str_eq("0", test_val_str);                                     
      test_val_str = NULL;                                                     
    }                                                                          

    {                                                                          
      test_val = INT_MAX;                                                      
      test_val_str = ip_itoa(test_val, buff, base);                            
      ck_assert_str_eq("2147483647", test_val_str);                            
      test_val_str = NULL;                                                     
    }                                                                          

    {                                                                          
      test_val = INT_MIN;                                                      
      test_val_str = ip_itoa(test_val, buff, base);                            
      ck_assert_str_eq("-2147483648", test_val_str);                           
      test_val_str = NULL;                                                     
    }                                                                          

    {                                                                          
      test_val = 50;                                                           
      test_val_str = ip_itoa(test_val, buff, base);                            
      ck_assert_str_eq("50", test_val_str);                                    
      test_val_str = NULL;                                                     
    }                                                                          

    {                                                                          
      test_val = -50;                                                          
      test_val_str = ip_itoa(test_val, buff, base);                            
      ck_assert_str_eq("-50", test_val_str);                                   
      test_val_str = NULL;                                                     
    }                                                                          
  } 
\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

I see a number of things that may help you improve your code.

Use the required #includes

The code uses strlen which means that it should #include <string.h>. It also needs stdlib.h, limits.h and more. The code is incomplete without the appropriate #includes.

Put static first

When declaring a variable or function, you should put the static keyword first. See this question for details on why.

Eliminate unused variables

Unused variables are a sign of poor code quality, so eliminating them should be a priority. In this code, carry_bit and complement_char are never used. My compiler also tells me that. Your compiler is probably also smart enough to tell you that, if you ask it to do so.

Be careful with signed versus unsigned

In the ncomp function, len is declared to be size_t type, but index is an int. They should probably both be size_t because they are compared within the for loops.

Practice safe programming

The ip_itoa function is passed a pointer to a buffer but not the length of that buffer, making it all too easy to create a buffer overflow. Pass the length as well and check it.

Order the functions

The ncomp function is called by ip_itoa and so either the ncomp should have a declaration above ip_itoa or the entire function should be moved above ip_itoa.

Eliminate spurious semicolons

The code currently contains this loop:

while (tmp) {
    tmp /= base;
    ++digit_count;
};   // <-- that semicolon is not needed

As the comment says, the final semicolon is not needed and should be eliminated.

Simplify your statements

The sizeof operator always returns 1 for sizeof(char) so statements like this:

return digit_count * sizeof(char);

should be simplified to just this:

return digit_count;

Fix the bugs

The program gives very strange output for negative numbers. For example, it says that -9 base 17 = "ggggggg8" but that corresponds to a value of 6975757432 which does not appear to be correct. Also, there are problems with other numbers. For example, it reports that 42 base 15 = "2<" which is clearly not correct. Here is the test program I used:

void checkConversion(int test_val, int base, char *buff) 
{
    char *test_val_str = ip_itoa(test_val, buff, base);
    long check_val = strtol(test_val_str, NULL, base);
    printf("%s: %d base %d = %s (check val = %ld)\n", 
            ((check_val == test_val) ? "OK " : "BAD"), 
            test_val, base, test_val_str, check_val);
}

#define VAL_COUNT 7
static const int vals[VAL_COUNT] = { -9, 100, 88, 0, -1, +1, 42};

int main() 
{
    int test_val;
    char *test_val_str;
    char *buff = malloc(100);
    for (int base = 2; base < 36; ++base) {
        for (int i = 0; i < VAL_COUNT; ++i) {
            checkConversion(vals[i], base, buff);
        }
    }
    free(buff);
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.