Why do I get a segmentation fault when using strncpy?

Question

I am getting segmentation fault when using strncpy and (pointer-to-struct)->(member) notation:

I have simplified my code. I initialise a struct and set all of it's tokens to an empty string. Then a declare a pointer to a struct and assign the address of the struct to it.

I pass the pointer to a function. I can print out the contents of the struct at the beginning of the function, but if I try to use the tp -> mnemonic in a strncpy function, I get seg fault. Can anyone tell me what I am doing wrong?

typedef struct tok  {
    char* label;
    char* mnem;
    char* operand;
}Tokens;

Tokens* tokenise(Tokens* tp, char* line)  {
    // This prints "load"
    printf("Print this - %s\n", tp -> mnem);

    // This function gives me segmentation fault
    strncpy(tp -> mnem, line, 4);

    return tp;
}

int main()  {
    char* line = "This is a line";
    Tokens tokens;
    tokens.label = "";
    tokens.mnem = "load";
    tokens.operand = "";

    Tokens* tp = &tokens;
    tp = tokenise(tp, line);

    return 0;
}

I have used printf statements to confirm that the code definitely stops executing at the strncpy function.

Answer 1

The problem is that tp->mnem is pointing to a string literal, which is generally allocated in a read-only segment of memory. Therefore it's illegal to overwrite it. Most likely what you need to do instead is something like this:

Tokens tokens;
tokens.label = "";
tokens.mnem  = strdup("load");
tokens.operand = "";

This will give you a dynamically allocated block of memory for mnem, which you can then write into as much as you like. Of course, you have a couple of other problems too: first, you'll need to remember to release that memory with free later; second, you'll have to be aware of the size of the buffer you've allocated so that you don't overwrite it.

If you know that the contents of mnem will never exceed 4 bytes, then you might instead change your structure declaration like so:

typedef struct tok  {
    char* label;
    char mnem[5]; // note: +1 byte for a NULL terminator
    char* operand;
}Tokens;

Then, you'd initialize it like this:

Tokens tokens;
tokens.label = "";
strcpy(tokens.mnem, "load");
tokens.operand = "";

This relieves you of the responsibility of managing the memory for mnem, although you still have some risk of overrunning your buffer.

Answer 2

Following line

tokens.mnem = "load"

assigns mnem to address of string literal, which is typically located in read-only data segment, so changing this memory with strncpy() or any other function will fail.

Answer 3

The problem is you've assigned string literals to the members of your Tokens structure and are trying to overwrite that memory (specifically, the mnem field) in tokenise.

Most modern OSes will allocate memory for string literals from a special read-only section of your program's address space. If you try to write to that memory, then your program will die with a segfault.

This is why the type of a string literal is const char *, not char *. Your compiler should warn you when you try to assign these to the fields of tokenise.

If you want to overwrite the memory later, you need to allocate the memory dynamically using malloc or change the members of the Tokens structure to fixed-length arrays, then copy the initial value into the allocated memory. Of course if you allocate the memory dynamically you need to free it later too.

Answer 4

You're calling strncpy() without having allocated the buffer spacem, just like Shadow said.

The literal string "load" you set the mnem member to in the initializer is not overwritable.

If you want to be able to change the string stored, and the size is reasonable, it might be easiest to just change the declaration of the struct field to char mnem[5];.

Also, please note that strncpy() has quite weird semantics. Check if you have strlcpy(); it's a better function.

Answer 5

You're getting a segmentation fault because this line:

strncpy(tp -> mnem, line, 4);

Is trying to copy four characters from 'line' into a location occupied by a string literal as assigned here:

tokens.mnem = "load";

The string literal is stored in a special text part of your program and may not be modified.

What you need to do is allocate a buffer of your own into which the string will be copied:

tokens.mnem = (char*) malloc (bufferSize);

And free the buffer when you are done using it.

Answer 6

This line is questionable:

strncpy(tp -> mnem, line, 4);

You are relying on a function that returns a pointer to memory that is not allocated. The return of *tokenise() is undefined. Its returning a pointer to memory that could contain all kinds of stuff, and that you don't have permission to modify.

It should return an allocated pointer.

Answer 7

You might malloc the tp variable. If you don't malloc there is no guarantee that the memory is actually yours. Don't forget to free the memory when you are finished.