CODEWITHSUNDEEP
c
Tal Marziano
Tal Marziano

Reputation: 21

"error reading characters of string" error after malloc for array of strings

Every time I try to allocate memory for array of strings, my program fails with an error message "error reading characters of strings". What am I doing wrong? Is there any problem with my memory allocation?

char** file2array(char *filename)
{
    FILE *fp = fopen(filename, "r");

    int i;
    int max_line_len = 20;
    int num_lines = 6;
    char** lines = (char **) malloc(sizeof(char*) * (num_lines + 1));

    fseek(fp, 0, SEEK_END);
    int chars_num = ftell(fp);
    rewind(fp);

    for (i = 1; i < (num_lines + 1); i++) {
        lines[i] = (char *) malloc(max_line_len + 1);
        fgets(lines[i], chars_num, fp);
    }

    fclose(fp);
    return lines;
}

Upvotes: 1

Views: 925

Answers (1)

John Bollinger
John Bollinger

Reputation: 180201

You appear to have not presented the code that actually emits the error message, so it's hard to be certain what it's complaining about. You certainly do, however, have a problem with your loop indexing:

    for (i = 1; i < (num_lines + 1); i++) {
        lines[i] = /* ... */

Variable i runs from 1 through num_lines, never taking the value 0, thus the first pointer in your dynamic array of pointers (at index 0) is never initialized. That's a plausible explanation for why some later code might have a problem reading the characters.

Additionally, you use fgets() wrongly. You tell it that your buffers are chars_num bytes long, but they are actually max_line_len + 1 bytes long. This presents the possibility of a buffer overrun.

Indeed, I'm uncertain what the point of chars_num is supposed to be. You initialize it to the length of the file (provided that that length fits in an int), but I'm not seeing how that's useful.

Furthermore, your function may return unexpected results if the file being read has lines longer than you account for (20 bytes, including line terminator) or a different number of lines than you expect (6).

I don't see any inherent problem with your memory allocation, but it wouldn't be StackOverflow if I didn't give you a severe tongue-lashing for casting the return value of malloc(). Consider yourself flogged.

Upvotes: 1

Related Questions