malloc array of structs + a little more

[kiser89]

I'm having a problem with my array of structs and segmentation faults. I have this struct that represents one line of a source file:

struct threeTokens {
int lineNumber;
char* cmd;
char* param;
}; line;

This is the code that tries to fill the array of structs, which is a global variable called program:

program = malloc(numLines * sizeof(line));
        while(NULL != fgets(buffer, SIZE, fp)){
                tokenPtr = strtok(buffer," ,\n");
                while(NULL != tokenPtr){
                        switch(count){
                        case 0: program[curLine].lineNumber = atoi(tokenPtr);
                                paramIndex += strlen(tokenPtr);
                                break;
                        case 1: program[curLine].cmd = (char*)malloc(strlen(tokenPtr) * sizeof(char));
                                strcpy(program[curLine].cmd, tokenPtr);
                                paramIndex += strlen(tokenPtr);
                                break;
                        default: paramLength += strlen(tokenPtr); break;
                        }
 
                        program[curLine].param = (char*)malloc(paramLength * sizeof(char));
                        for(i = 0; i < paramLength; i++){
                                program[curLine].param[i] = buffer[paramIndex + i];
                        }
                        tokenPtr = strtok(NULL, " \n");
                }
                curLine++;
        }/*End While of buffer*/

When I try:

for(i = 0; i < numLines; i++){
        printf("%d\t%s\t%s\n",program[i].lineNumber, program[i].cmd, program[i].param);
}

I get:

0       (null)
0       (null)
0       (null)
0       (null)
0       (null)

This results in a segmentation fault later on in my code which I'm sure is from me trying to access some null data. Any ideas?

[weaknessforcats]

Afyer you allocate the threeToken array, there are pointers (cmd and param) that are allocated but contain no address.

You need to allocate the memory those pointers point at.

[kiser89]

Isn't that what I do within my switch statement? I malloc the memory and then copy the token into the newly allocated memory, right?

[weaknessforcats]

strcpy(program[curLine].cmd, tokenPtr);

Where have you allocated program[curLine].cmd?

strcpy will copy from tokenPtr to program[curLine].cmd until it finds a \0.

You do not own the address in program[curLine].cmd since you never allocated it and that's where your seg error comes from.

You need to allocate strlen(tokenPtr) +1 and put that address in program[curLine].cmd before you strcpy.

[kiser89]

One line above that I have:

program[curLine].cmd = (char*)malloc(strlen(tokenPtr) * sizeof(char));

I may need a '+1' but that doesn't explain why the entire string at program[curLine].cmd is null. At least I don't think, maybe I'm still missing what you're trying to say. But it's my understanding that line allocates the memory I need to copy tokenPtr into cmd.

[kiser89]

Anyone else? This is very frustrating :(

[newb16]

Did you try it with '+1' ? Anyway, without reproducible example we can't help you much - you can try to step it through in debugger and see where the string that is supposed to be there corrupts, or put debug prints there for, e.g., program[0].cmd withing the loop to detect when it corrupts.

[Banfa]

What I see is that the calculation of the token size at lines 9 and 13 using sizeof both omit to add 1 (+1) to take account of the NULL terminator so your allocated buffers are too small.

You are doing something strange with program[curLine].param in that you don't treat it like program[curLine].cmd and just allocate and copy it. I am guessing that you are string to copy everything after the first 2 tokens into it but you actually allocate and repeated copy to it causing multiple memory leaks and on cases 0 and 1through the loop quite possibly having a paramLength == 0 therefore causing invalid memory accesses by dereferencing the NULL pointer.

And finally the 2 lines that call strtok 3 and 20 use different separators which seems unlike to be correct and both include the \n character which certainly isn't correct since fgets reads a single line from the file there can be no more than a single \n of the line read. When parsing a line it is best to parse in a whitespace agnostic fashion and then strip leading and trailing white spaces from the tokens before storing them.


All the logic errors and possibilities for undefined behaviour in this code seem more than enough to me to result in the output you have.

[RRick]

I'll throw my 2 cents in and notice that count in the switch statement is never incremented. Each time you loop through you will do the same thing over and over (I assume count is initialized to 0) until you run out of tokens from strtok.

You are also doing something "weird" with the buffer by maintaining an offset paramIndex that you keep increasing. Finally, paramLength is not set until the default case, and who knows what is being copied.


What I notice in the code is that you are fighting against what is being returned by strtok. That might explain the paramIndex value. The simplest solution is to have strtok do the work for you and you control this with the delimiters passed to strtok. When strtok finds what you need, you simply copy the data from tokenPtr to your structure. There is no need to maintain an index into the buffer.