Mysterious segmentation fault while creating a Linked List (Add function)

Question

Im trying to create a Linked List in C but the program crashed due to some mysterious fault.

First I tried this:

typedef struct product_data product_data;

struct product_data {
 int product_code;
 int product_size;
 product_data *next;
};

product_data *products_head = NULL;
product_data *products_tail = NULL;

int main() {
    int newcode = 5;
    int newsize = 5;
    products_head->product_code = newcode;
    products_head->product_size = newsize;
    products_head->next = NULL;

    return 0;
}

Unfortunately the program crashes without any error message.

Then I changed some parts:

typedef struct product_data product_data;

struct product_data {
 int product_code;
 int product_size;
 product_data *next;
};

product_data *products_head = NULL;
product_data *products_tail = NULL;

int main() {
    product_data *newproduct;
    int newcode = 5;
    int newsize = 5;
    newproduct->product_code = newcode;
    newproduct->product_size = newsize;
    newproduct->next = NULL;

    products_head = newproduct;

    return 0;
}

No crash this time, it seems to work. I have no idea why though.

Any ideas?

Thanks in advance!

Have to disappoint you -- no homework. ;-) Unfortuantely Im not that young anymore ... — user1438181
– user1438181, Commented Jun 6, 2012 at 17:45
Didn't mean to insult; these questions just come "in batch" sometimes :-) — user166390
– user166390, Commented Jun 6, 2012 at 18:21

Daniel Fischer · Accepted Answer · 2012-06-05 19:42:27Z

4

It doesn't really work. You're still dereferencing invalid pointers:

product_data *newproduct;
int newcode = 5;
int newsize = 5;
newproduct->product_code = newcode;
newproduct->product_size = newsize;
newproduct->next = NULL;

But while in the first version you were dereferencing pointers explicitly set to NULL, it crashed with a segmentation fault like it should. Here you are dereferencing a pointer that contains whatever data lay on the stack, and unfortunately it doesn't crash. It's undefined behaviour, so it need not necessarily crash.

You have to let your pointers point to valid memory,

newproduct = malloc(sizeof product_data);

answered Jun 5, 2012 at 19:42

Daniel Fischer

185k19 gold badges319 silver badges436 bronze badges

Sign up to request clarification or add additional context in comments.

Comments

Yusuf X · Accepted Answer · 2012-06-05 19:42:08Z

1

You need to allocate memory for products_head. Right now you're just setting it to NULL. Either don't make it a pointer, or use malloc.

answered Jun 5, 2012 at 19:42

Yusuf X

14.6k5 gold badges37 silver badges48 bronze badges

Comments

Keith Randall · Accepted Answer · 2012-06-05 19:42:41Z

1

In your first sample, you are writing to a NULL pointer. You need to allocate space for products_head before you dereference it. Something like

products_head = malloc(sizeof(product_data));

I don't know why your second example worked. It shouldn't. newproduct is an uninitialized variable, it could point anywhere. Maybe you just got lucky and it pointed to an unused chunk of valid memory.

answered Jun 5, 2012 at 19:42

Keith Randall

23.3k3 gold badges38 silver badges54 bronze badges

Comments

Virtually Real · Accepted Answer · 2012-06-05 19:45:37Z

0

This will work until it does not. You still do not have any allocated memory for your struct. But due to some luck, newproduct points to some memory location that is valid. The issue you are facing, product_head was manually set to null (even though this is not needed, since all global variables are always initialized). Stack variables however are not initialized, and you were lucky (or unlucky that it would have caused you to miss a blatant programming mistake) that it happened to point to somewhere valid in your address space.

You can print the content of newproduct to see where it is pointing using printf("%p", newproduct); Unfortunately, inserting this line may change the behavior of the program.

answered Jun 5, 2012 at 19:45

Virtually Real

1,6954 gold badges18 silver badges20 bronze badges

Comments

bktz · Accepted Answer · 2012-06-05 19:53:40Z

The "->" is meant to access an element in a structure that is dynamically allocated and "." is used to access an element in a structure that is statically allocated.

Here is an example:

typedef struct product_data product_data;

struct product_data {
    int product_code;
    int product_size;
    product_data *next;
};

product_data *products_head = NULL;
product_data *products_tail = NULL;

int main() {
    /* Allocate memory */
    product_data *newproduct = malloc(sizeof(product_data));
    int newcode = 5;
    int newsize = 5;

    products_head = newproduct;
    newproduct->product_size = newsize;
    newproduct->next = NULL;

    /* free memory */
    free(product_data);

    return 0;
}

But remember that for all the new nodes that you make in the linked list you will have to allocate memory and free that memory. A good program to use to check that all the memory you allocated was free'd is valgrind. And if you run into logic errors trying to make the linked list draw it out by hand first like this:

      [head]   [tail]
        |        |
        V        V
      [ a ] -> [ b ] -> null

Just remember that head and tail are both pointers (so they do not need to be allocated memory, they just need to be POINTING at the node that you want.

If you still run into problems because your logic gets very complicated I sugget you try and learn GDB (it is a command line debugger) it will help you step through your code so you can see what is happening step by step. That is how I first learnt to create data structure.

Good Luck!

Collectives™ on Stack Overflow

Mysterious segmentation fault while creating a Linked List (Add function)

5 Answers 5

Comments

Comments

Comments

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

5 Answers 5

Comments

Comments

Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related