1

I have the following code:

struct argument {
    char *source;
    char *destination;
    int value;
};

int main(void){
    struct argument *arg2 = malloc(sizeof(struct argument));
    strcpy(arg2->source, "a1"); //This gives segmentation fault.
    strcpy(arg2->destination, "a2");
    arg2->value = 1500;

    return 0;
}

The problem is that when i use strcpy with arg2->destination, everything runs fine, but as soon as I uncomment the strcpy with arg2->source, I immediately get a segmentation fault. What is wrong? Is there a way to fix this?

Pengibaby
  • 373
  • 2
  • 11
  • @uneven_mark thank you for your reply! But why is it that when i comment out the first strcpy, but keep the second one, the program still runs without a problem? – Pengibaby Nov 03 '19 at 02:32
  • 2
    Does this answer your question? [Crash or "segmentation fault" when data is copied/scanned/read to an uninitialized pointer](https://stackoverflow.com/questions/37549594/crash-or-segmentation-fault-when-data-is-copied-scanned-read-to-an-uninitializ) – walnut Nov 03 '19 at 02:32
  • right, but still a question why destination works fine, only source fails – Anatoliy R Nov 03 '19 at 02:33
  • 4
    To repeat my (accidentally) deleted comment: You never allocate memory for the `source` and `destination` pointers to point to and for `strcpy` to copy the literals to. – walnut Nov 03 '19 at 02:33
  • 2
    @Pengibaby / #AnatoliyR Using an uninitialized pointer in `strcpy` causes [undefined behavior](https://stackoverflow.com/questions/2397984/undefined-unspecified-and-implementation-defined-behavior). Undefined behavior means *anything* can happen, it can even seem to work randomly. There is no guarantee that you will get an error message. – walnut Nov 03 '19 at 02:34
  • Your argument struct has room for two pointers and an int, you need initialize pointers to somewere otherwise you're dereferencing uninitialized memory, and boom ... It segfaults – geckos Nov 03 '19 at 02:37
  • @uneven_mark makes sense, this is run-time thing... lack of practice in C last several years – Anatoliy R Nov 03 '19 at 02:37
  • Thank you for the help! So I should use malloc(sizeof(char*)) to allocate memory for each of the char pointers as well after allocation memory for the struct? – Pengibaby Nov 03 '19 at 02:48
  • @Pengibaby Please read the answers in the linked duplicate. You need to allocate for as many `char`s as the string you want to copy into it is long, taking into account the null-terminator of the string. – walnut Nov 03 '19 at 02:51

2 Answers2

4

You're a victim of the all-too-famous undefined behavior. You see, when uninitialized pointers are used in calls to strcpy, there's no guarantee that it will work nor that it will throw an error. Basically, anything can happen, it can even work as expected sometimes, which makes this type of bug even more tedious to find and correct.

You need to allocate memory for your source and destination pointers before using them, you can use malloc(strlen("someString") + 1);. And don't forget to release the memory when you're done by calling free. Adopting this habit early on will free you in the future of some very nasty bugs, and more undefined behavior.

Javier Silva Ortíz
  • 2,864
  • 1
  • 12
  • 21
2

with this statement struct argument *arg2 = malloc(sizeof(struct argument)); you allocate the memory for the structure itself. The struct contains the following fields:

struct argument {
    char *source;  // << a pointer to a character string
    char *destination; // << another pointer
    int value; // integer value
};

Next you are trying to copy a string, using one of the pointers: strcpy(arg2->source, "a1");. But ... there is no place for the string in memory. You allocated place for the pointer (a part of the struct) but not for the string which it would point to. It causes memory corruption and your crash.

So, before copying the string, allocate some place for it:

arg2->source = malloc(3); 

you need '3' which is 1 char longer than the length of the string 'a2'.

same for the other string.

You do not need to do anything extra for the value. It is not a pointer but a part of the struct, which have already allocated.

Serge
  • 11,616
  • 3
  • 18
  • 28