segfault when reading last line of file using fgets()

Question

I'm solving a coding problem for a Data Structures assignment, and one part of my solution keeps giving me a segfault: 11 when reading the last line of a text file using fgets().

I've already searched around StackOverflow for some time now and can't find the same error I'm getting. I've tried many ways to solve the problem but I can't figure out what exactly is wrong.

int main() {
    **S = func();
    free(S);
    return 0;
}

char** func() {
    FILE *fStrings = fopen("file.txt", "r");
    if (fStrings == NULL) printf("Error opening 'file.txt'\n")
    int length = fileLength("file.txt");  // This returns just the number of lines in a file, works as intended.
    char **strings = (char **)malloc(200 * length * sizeof(char));
    f(length, (char (*)[200])strings, *fStrings);
    // some code using the data modified by f()
    fclose(fStrings);
    return strings;
}

void f(int length, char strings[][200], FILE *fStrings) {
    int i;
    for (i = 0; i <= length; i++) {
        printf("i = %d\n", i);  // debug
        fgets(strings[i], 200, fStrings);  // Here's the problem
        printf("string = %s\n", strings[i]);  // debug
    }
}

The above code has 2 debug lines to see where exactly the error happens. The function is called with the correct parameters, where length is the amount of strings in the array, strings is an array of strings, and fStrings a file with said strings.

The segfault occurs when trying to read the last line of the text file. Any help will be appreciated.

EDIT: Changed the code block to include a better version of my code, in case it makes it easier to understand. The correct libraries are included too.

Your pointer calculations won’t work. Why not use the arrays properly, like strings[i] as you do in the printf anyway? Where did you learn to do this, out of curiosity? — Sami Kuhmonen
@SamiKuhmonen The pointer calculation is strange but why won't it work? — 4386427
You need to show us how you call the function and how the parameters are defined and initialized. — 4386427
@4386427 Because an array of arrays is not necessarily in contiguous memory. They can be anywhere so accessing past the end of one isn’t ok. — Sami Kuhmonen
for (i = 0; i <= length; i++) { ==> for (i = 0; i < length; i++) { — 4386427

4386427 4386427 · Accepted Answer · 2019-03-30T19:18:05

I think the main problem with your code is this line

for (i = 0; i <= length; i++) {

Due to <= it will loop "length + 1" times but you have only allocated memory for "length" times. Change it to:

for (i = 0; i < length; i++) {

Your allocation is strange. You should allocate a 2D array using a pointer to a 1D array, i.e. like this:

char (*strings)[200] = malloc(length * sizeof *strings)

then you can do the call without any cast.

Also just noticed this line:

f(length, (char (*)[200])strings, *fStrings);
                                  ^
                                  notice

I don't think you want * in front of fStrings (it should not even compile with the *)

With the correct allocation (as described above) the call should be:

f(length, strings, fStrings);

segfault when reading last line of file using fgets()

1 Answers