Segmentation fault when calling fgets to read lines from file

Question

I'm getting a seg fault when after calling fgets about 20 times. I'm opening a file (does not return null). It is in the format:

 num1: value1
 num2: value2
 num3: value3

then reading lines from the file, storing values into an array, using nums as positions. Here is the code that seg faults:

edit: declaring myArray and line:

char myArray[3000];    
char * line;
char * word;

line = (char *) malloc(100);
word = (char *) malloc(16);

while(fgets(line, 99, file)) {
    printf("%s\n", line);
    word = strtok(line, " :");
    name = (int) strtol(word, NULL, 16);

    word = strtok(NULL, " \n");
    myArray[name] = word;
}

you'll notice I print out the line immediately after getting it. The file has 26 lines, but it only prints 23 line and then seg faults. Now, is it something I don't fully understand about fgets, or am I getting some synthax incorrect? I've tried allocating more memory to line, or more to word. I've also tried malloc -ing more memory after every call to strtok, but nothing seems to fix the seg fault.

Can you post more of your code? For example, what is the declaration of myArray? — Baldrick
There's no error checking on the return value from strtok nor is there bounds checking on the length of word. Either could easily account for a seg fault. Have you stepped through the code with a debugger? That would likely tell you immediately where the problem lies. — Carey Gregory
It would be extremely strange for your debugger not to hit the same issue... did you run it on the exact same input files (both with and without the debugger)? By the way, you're assigning a char * to a char in myArray[name] = word. I'm pretty sure C takes the lowest byte of the RHS in that assignment. — rliu
while(fgets(line, 99, file) != EOF) is not correct... fgets() never returns EOF, it returns either the pointer you passed in or NULL what you need to do is make the inside of the loop deal with empty or badly formatted lines, then the check you have is fine. — Spudd86

Spudd86 Spudd86 · Accepted Answer · 2013-10-29T04:14:09

The problems is the line myArray[name] = word; you're taking an array index from your input line and then setting the character at that position to low bits of the address of your word... I doubt that's actually what you want to do.

There's some other problems with your code, you're leaking the memory from the line word = (char *) malloc(16); because strtok returns a pointer into the string you initially pass it. You don't actually need to malloc anything for the code as written in the question, so you could have:

char myArray[3000];    
char line[100];
char *word = NULL;

word needs to be a pointer since it's holding the result of strtok()

You clearly don't understand pointers, you need to review that before you can understand why your code isn't working the way you expect.

If you say what your code is actually meant to be doing I can give you some hints on how to fix it, but at the moment I can't quite tell what the intended result is.

EDIT: Did you intend to read in your numbers in hexadecimal? The last argument to strtol() is the base to be used for conversion... you could also just use atoi()

so your loop could look like:

char myArray[3000];    
char line[100];
char *word = NULL;


while(fgets(line, 100, file)) {
    printf("%s\n", line);
    word = strtok(line, " :");
    if(word == NULL) continue;
    name = atoi(word); /* only if you didn't actually want hexadecimal */

    word = strtok(NULL, " \n");
    if(word == NULL) continue;

    if(name > 0 && name < 3000) { /* as I said in a comment below */
        strncpy(myArray + name, word, 3000 - name);
    }
}

Segmentation fault when calling fgets to read lines from file

1 Answers