Realloc array of Strings in C? segmentation error

Question

#include <stdlib.h>
#include <stdio.h>
#include <string.h>
void sortString(const char* input, char* output);
int cmpstr(void const *a,void const *b);
int readAllWords(FILE* f, char*** res, int * num_read);
int main (int argc, char ** argv)
{
   char **wordList;
   FILE* fid;
   int numWords;
   fid = fopen(argv[1],"r");
  readAllWords(fid, &wordList,&numWords);
}

int readAllWords(FILE* f, char*** res, int * num_read)
{
    char buffer[128];
    *num_read = 0;
    int size;
    while(fgets(buffer,128,f))
    {
       *num_read = *num_read +1;
       size = strlen(buffer);
       res = (char***)malloc(sizeof(char**));
       *res = (char **)realloc(*res,sizeof(char*)*(*num_read));
       (*res)[(*num_read)-1] = (char *)realloc((*res)[(*num_read)-1],sizeof(char)*size);
        strcpy((*res)[(*num_read)-1],buffer);
      printf("%s\n",(*res)[(*num_read)-1]);
    }
    printf("%s\n",(*res)[0]);

}

The values are storing and it prints out inside the while loop. But after the while loop, it cannot print out the strings.

The File is given in the main function. Do not understand why realloc is causing the loss of data?

`res = (char***)malloc(sizeof(char**));` should be an alert as to the fact that you are doing it wrong. — , Jan 19 '14 at 23:57
[Do **NOT** cast the return value of `malloc()`](http://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc/605858#605858)! — , Jan 19 '14 at 23:58
Looks like you aren't allocating an extra spot for the \0 in the realloc, but I agree with @H2CO3, `(char***)` is a sign that you need to refactor... — John3136, Jan 20 '14 at 00:01
The thing is that I cannot change the function parameters so what is the proper way to initialize an array of strings? — user3213348, Jan 20 '14 at 00:04

score 1 · Answer 1 · 2014-01-20T00:21:07.170

1

Possible reason for a segmentation fault:

 res = (char***)malloc(sizeof(char**));
 *res = (char **)realloc(*res,sizeof(char*)*(*num_read));

In the second line you try to reallocate whatever *res is pointing to. However since you did not initialize *res this could be anything. This will work only if *res == NULL. I guess it should be malloc, not realloc.

Other problems:

You allocate everything new in each loop iteration. This is a huge memory leak.

You already pass a valid memory address pointing to an char** by res, you shouldn't allocate for it again. It is an out parameter. (Remove the malloc call)

You need an initial malloc for *res before the loop (Or set *res = NULL).

The second realloc for *res[...] should be a malloc, because you never actually reallocate here. Also instead of allocating size bytes, you should allocate size+1 bytes for the terminating \0.

Your function has no return statement although it is non-void.

edited Jan 20 '14 at 00:21

answered Jan 20 '14 at 00:01

But I need to increase the size of the array of pointers bc it is an array of strings. – user3213348 Jan 20 '14 at 00:08
do i need not set up **res for the character array? – user3213348 Jan 20 '14 at 00:09
@user3213348: That pair of lines is another problem. Yet another problem is that you only allocate enough space for `size` characters, not `size+1`. Always remember to account for the null byte at the end of a string; `strlen()` does not count that byte. – Jonathan Leffler Jan 20 '14 at 00:47

Jonathan Leffler · Accepted Answer · 2014-01-20T01:19:19.853

One problem is that the code doesn't initialize res in main(), so you attempt to realloc() an indeterminate value. Either NULL or a value previously returned by malloc() or realloc() (or calloc()) would be OK, but since you pass an indeterminate value, you are invoking undefined behaviour, and a crash is a valid response to doing that.

However, there's a lot of other code in the function that should be reviewed as well.

This code works, and gets a clean bill of health from valgrind.

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

void readAllLines(FILE *f, char ***res, int *num_read);

int main(int argc, char **argv)
{
    char **wordList = 0;
    FILE *fid;
    int numLines = 0;
    if (argc > 1 && (fid = fopen(argv[1], "r")) != 0)
    {
        readAllLines(fid, &wordList, &numLines);
        fclose(fid);
        for (int i = 0; i < numLines; i++)
            printf("%d: %s", i, wordList[i]);
        for (int i = 0; i < numLines; i++)
            free(wordList[i]);
        free(wordList);
    }
    return 0;
}

void readAllLines(FILE *f, char ***res, int *num_read)
{
    char buffer[128];
    int size;
    while (fgets(buffer, sizeof(buffer), f))
    {
        *num_read = *num_read + 1;
        size = strlen(buffer) + 1;
        char **space = (char **)realloc(*res, sizeof(char *) * (*num_read));
        if (space == 0)
            return;
        *res = space;
        (*res)[*num_read - 1] = (char *)malloc(sizeof(char) * size);
        if ((*res)[*num_read - 1] == 0)
            return;
        strcpy((*res)[*num_read - 1], buffer);
        printf("%s\n", (*res)[*num_read - 1]);
    }
    printf("%s\n", (*res)[0]);
}

Realloc array of Strings in C? segmentation error

2 Answers2