0

I can't seem to figure out why strcpy makes a segmentation fault in this code. It should be straightforward:

    typedef struct message {
        char *buffer;
        int length;
    } message_t;

    int main () {
        char* buf = "The use of COBOL cripples the mind; its "
                    "teaching should, therefore, be regarded as a criminal "
                    "offense. -- Edsgar Dijkstra";
        message_t messageA = {"", 130};
        message_t *message = &messageA;
        strcpy(message->buffer, "buf");
        printf("Hello\n");
    }

EDIT: strcpy(message->buffer, "buf") is supposed to be strcpy(message->buffer, buf) without the "" quotes

EDIT: Thank you to the comments! This has been resolved by malloc'ing message->buffer to make space for buf:

    message_t messageA = {"", 130};
    message_t *message = &messageA;
    message->buffer = malloc(122);
    strcpy(message->buffer, buf);
    printf("Hello\n");
Anatoly
  • 20,799
  • 3
  • 28
  • 42
Faris Durrani
  • 87
  • 3
  • 8
  • 4
    `message->buffer` is a pointer, so should be `malloc`d, also i guess you want to use `buf` ptr variable not as a string "buf" – csavvy Nov 28 '20 at 16:53
  • 2
    `message->buffer` points to the string literal `""`, since that is how you initialized `messageA`. So your `strcpy` is attempting to write over a string literal, which is always undefined behavior. You need to have `message->buffer` pointing instead to a block of memory with enough space for the string and its trailing null byte. – Nate Eldredge Nov 28 '20 at 16:55

3 Answers3

2

some points to be noted here.

when you declare pointers to store data you either assign directly at the declaration (usually used for small strings not big strings) or you should allocate memory using dynamic memory allocation functions

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

typedef struct message {
    char *buffer;
    int length;
} message_t;

int main () {
    char* buf = "The use of COBOL cripples the mind; its "
                "teaching should, therefore, be regarded as a criminal "
                "offense. -- Edsgar Dijkstra";
    message_t messageA = {"", 130};
    message_t *message = &messageA;

    //allocate memory to buffer length of buf, + 1 for \0 character
    message->buffer = malloc( strlen(buf) + 1 );
    // check allocated memory is success or not
    if ( message->buffer )
    {
        strcpy(message->buffer, buf);
        printf("buffer = %s\n",message->buffer );
        //free the malloc'ed memory to avoid memory leaks
        free(message->buffer);
        //make the pointer NULL, to avoid dangling pointer
        message->buffer = NULL;
    }
    else
    {
        printf("malloc failed\n");
    }
    printf("Hello\n");
    return 0;
}
csavvy
  • 761
  • 4
  • 13
1
message_t messageA = {"", 130};

Here, you initializing messageA.buffer = "". It is a string literal. So, you cannot modify the string stored in it. If you try to modify it, you will get segmentation fault.

message_t *message = &messageA;
strcpy(message->buffer, buf);

Here, you are modifying the string literal message->buffer. That's why you got segmentation fault.
Please visit this question Modifying a string literal

Krishna Kanth Yenumula
  • 2,533
  • 2
  • 14
  • 26
0

Try using message->buffer = strdup(buf); that does the malloc and strlen computation for you.

Vercingatorix
  • 1,838
  • 1
  • 13
  • 22