Question

I am having trouble reading a file into an array of strings in C. Here is my code:

  char mylittleBuffer[BUFFER_SIZE];     //BUFFER_SIZE is 4096
  char *a;
  char **c;

  fprintf(stderr, "LOL\n");
  for(int i = 0; (a = fgets(mylittleBuffer, sizeof(mylittleBuffer), myInput)) != NULL; i++)
    {
      fprintf(stderr, "a: %s\n", a);
      c[i] = malloc(sizeof(a));
      if (c[i] == NULL)
        printf("c[i] is NULL");
      c[i] = strdup(a);
      //      fprintf(stderr, "mylittleBuffer: %s\n", mylittleBuffer);                                                                                                                                                   
      fprintf(stderr, "c[i] %s\n", c[i]);
    }

It appears that only one line of the file is read as the program will print out a once. What happened to the other lines? I get no error messages...

  1. What is the issue here?
  2. How can I fix this problem?
Was it helpful?

Solution

You have not initialized c to point to anything (you need to allocate space for it), so when you use c[i], you are using undefined memory locations, invoking undefined behaviour. It is a miracle that it does not crash. You will need to allocate space for the array of character pointers.

enum { INITIAL_SIZE = 2 };            // Set to a larger number when not debugging
char mylittleBuffer[BUFFER_SIZE];     //BUFFER_SIZE is 4096
char *a;
size_t c_size = INITIAL_SIZE;
char **c = malloc(c_size * sizeof(*c));

if (c == NULL)
{
    fprintf(stderr, "out of memory (for c)\n");
    return;
}

fprintf(stderr, "LOL\n");
for (int i = 0; (a = fgets(mylittleBuffer, sizeof(mylittleBuffer), myInput)) != NULL; i++)
{
    fprintf(stderr, "a: %s\n", a);
    if (i >= c_size)
    {
         // Reallocate c to get more space
         size_t new_size = c_size * 2;
         void *new_space = realloc(c, new_size * sizeof(*c));
         if (new_space == 0)
         {
             // Release the already allocated c[i]
             // Release c
             fprintf(stderr, "Out of memory (for more c)\n");
             return;
         }
         c_size = new_size;
         c = new_space;
    }
    // c[i] = malloc(sizeof(a));  // Leak - you use strdup() too
    c[i] = strdup(a);
    if (c[i] == NULL)
    {
        fprintf(stderr, "c[i] is NULL\n");
        // Release the already allocated c[i] strings
        // Release c
        return;
    }
    //      fprintf(stderr, "mylittleBuffer: %s\n", mylittleBuffer);
    fprintf(stderr, "c[%d] <<%s>>\n", i, c[i]);  // <<>> show where the string ends
}

I've kept your code mostly. Were it mine, a would not exist, mylittleBuffer would be just buffer, and that's what I'd use in the body of the loop in place of a. I'd probably use a longer name for c, though I usually use shorter names than other people do.

Note how the code using realloc() avoids losing the pointer to the previously allocated memory if the call fails. If it assigned directly to c, you'd lose the only reference to the previous memory allocation — a possibly major leak.

Given that the clean-up code is needed twice, I'd write a function to do it:

static void release_memory(size_t num, char **strings)
{
    for (size_t i = 0; i < num; i++)
        free(strings[i]);
    free(strings);
}

It would be called as:

release_memory(i, c);
Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top