0
\$\begingroup\$

This function creates takes an int * buffer and creates a neatly formatted string (useful for printing the contents of an array).

  • Is the code easy to follow?
  • Is it efficient?
  • Am I allocating and freeing memory properly?
  • Bonus question: is there an idiomatic way to do this in C, such as through a set of library routines?
#include <stdlib.h>
#include <stdio.h>
#include <string.h>

char *int_array_to_string(int *arr, size_t length)
{
    char **strings = calloc(length, sizeof(char *));
    int num_chars = 0;
    for (size_t i = 0; i < length; i++)
    {
        char *num_string = calloc(32, sizeof(char)); // 32 digits should fit any int
        sprintf(num_string, "%d", arr[i]);
        strings[i] = num_string;
        num_chars += strlen(num_string);
    }
    num_chars += 2 * (length - 1); // extra bytes for comma and space chars following the numbers (except for the last number)
    num_chars += 2;                // bytes for curly braces at the beginning and end

    char *result = calloc(num_chars, sizeof(char));
    size_t i = 0;
    result[i++] = '{';
    for (size_t j = 0; j < length; j++)
    {
        char *str = strings[j];
        for (size_t k = 0; k < strlen(str); k++)
            result[i++] = str[k];
        free(str);
        result[i++] = ',';
        result[i++] = ' ';
    }
    free(strings);
    result[num_chars - 1] = '}';

    return result;
}
\$\endgroup\$

1 Answer 1

1
\$\begingroup\$
  • result is not zero-terminated. You should allocate one byte more.

  • To my taste, there are too many allocations. Compute the required size, and allocating once. You already allocate 32 bytes per array element, so

      char * result = malloc(length * 32 + whatever_extra_space_necessary);
    
  • Now recall that sprintf returns an amount of bytes it actually printed, and remove a superfluous call to strlen:

      char * where = result;
      for (size_t i = 0; i < length; i++) {
          size_t printed = sprintf(where, "%d, ", arr[i]);
          where += printed;
      }
    
  • Your code prints an unpleasant ", " after the last element of an array. If it is a conscious decision, it is all right; if it is not, consider printing the first element separately:

      print("%d", arr[0]);
      for (i = 1; ....)
          print(", %d", arr[i]);
    
\$\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.