Trouble dynamically allocating memory for string array

Question

I am trying to write a function that reads a text file and copies each line of the text file into a line of an array that is passed into the function.

void read_lines(FILE* fp, char*** lines, int* num_lines) {  
    int i = 0, line_count = 0;
    char line[256], c;

    fscanf(fp, "%c", &c);
    while(!feof(fp)){
        if(c == '\n') {
            ++line_count;
        }
        printf("%c", c);
        fscanf(fp, "%c", &c);
    }

    rewind(fp);
    *num_lines = line_count;

    lines = (char***)malloc(line_count * sizeof(char**));
    while (fgets(line, sizeof(line), fp) != NULL) {
        lines[i] = (char**)malloc(strlen(line) * sizeof(char*));
            strcpy(*lines[i], line);
        }
        ++i;
    }
}

The initial part scans for newlines so that I know how much to allocate to lines initially. I am not sure where I am going wrong.

Additionally, if anybody has any resources that could help me to better understand how to dynamically allocate space, that would be greatly appreciated.

[don't cast malloc in C](http://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc) — Barmar, Nov 15 '16 at 03:24
[Don't cast the result of `malloc` in C](http://stackoverflow.com/q/605845/995714). [What are the barriers to understanding pointers and what can be done to overcome them?](http://stackoverflow.com/q/5727/995714) — phuclv, Nov 15 '16 at 03:25
You need to assign to `*lines` to modify the caller's variable. That's why you have the triple pointer. — Barmar, Nov 15 '16 at 03:25
valgrind is a good tool to debug some memory problems. Or a debugger. — , Nov 15 '16 at 03:26
`*lines = malloc((line_count+1) * sizeof(char*));`, `(*lines)[i] = malloc(strlen(line) +1);`, `strcpy((*lines)[i], line);` — BLUEPIXY, Nov 15 '16 at 03:28

score 2 · Accepted Answer · answered Nov 15 '16 at 03:36

You should understand how the pointers work. After that, dynamical memory allocation task would be pretty trivial. Right now your code is completely wrong:

//here you assign to the argument. While this is technically allowed 
//it is most certainly not what you have intended
lines = (char***)malloc(line_count * sizeof(char**));
while (fgets(line, sizeof(line), fp) != NULL) {
        //sizeof(char*) <> sizeof(char).  Also you need a space for the trailing \0 
        lines[i] = (char**)malloc(strlen(line) * sizeof(char*));
        //[] precedes * so it is copying the string somewhere you not intend to
        strcpy(*lines[i], line);
    }
    ++i;
}

Correct version should be:

*lines = malloc(line_count * sizeof(char*));
while (fgets(line, sizeof(line), fp) != NULL) {
    (*lines)[i] = malloc((strlen(line) + 1) * sizeof(char));
    strcpy((*lines)[i], line);
    }
    ++i;
}

Note, that you need to use (*lines)[i] construct, because [] operator precedes * (dereference) operator.

score 1 · Answer 2 · edited May 23 '17 at 12:24

Code is making various mistakes including the key ones detailed by @Ari0nhh

The other is the counting the '\n' can fail to get the correct number of lines vs. fgets() in 3 ways:

Line exceeds 256.
More than INT_MAX lines.
Last line does not end with a '\n'.

Suggest instead use the same loop to count "lines"

unsigned long long line_count = 0;
while (fgets(line, sizeof(line), fp) != NULL) {
    line_count++;
}

rewind(fp);

.... 

assert(line_count <= SIZE_MAX/sizeof *(*lines));
*lines = malloc(sizeof *(*lines) * line_count);

Trouble dynamically allocating memory for string array

2 Answers2