Segmentation fault in C

Question

I have the following C program. It works when I include the following line, otherwise it gives a segmentation fault:

printf("head(%p), last(%p), newnode(%p)\n", head, last, newnode);

Any idea what's the issue here?

Here is my whole program. It's a basic circular queue example.

#include "stdio.h"
#include "malloc.h"

struct node {
    int data;
   struct node *next;
};

typedef struct node NODE;

void display(NODE *);

int main(void) {

    NODE *head, *last, *newnode = NULL;
    int i = 5;

    for ( ; i > 0; i--) {
        newnode = (NODE *) malloc(sizeof(NODE));
        newnode->data = i*10;
        newnode->next = NULL;

        //printf("head(%p), last(%p), newnode(%p)\n", head, last, newnode);


        if (head == NULL) {
            head = newnode;
            last = newnode;
        } else {
           last->next = newnode;
           last = newnode;
        }

        last->next = head;
    }

    display(head);

    return 1;
}

void display(NODE *head) {

    NODE *temp = NULL;
    temp = head;

    printf("Elements --> ");
    do {
            printf("%d ", temp->data);
            temp = temp->next;
    } while (temp != head);

    printf("\n");

}

pb2q · Accepted Answer · 2012-07-08 22:27:06Z

7

You need to explicitly initialize head and last to NULL also.

In your declaration:

NODE *head, *last, *newnode = NULL;

only newnode is being initialized to NULL. Consequently later in your if/else you're testing/assigning to random memory.

Do something like this:

NODE *head, *last, *newnode;
head = last = newnode = NULL;

You can't assume that pointers are automatically initialized to NULL, even though it may be the case on certain systems. Variables declared as static are an exception. Always initialize variables in C to reasonable values, especially pointers.

When you access garbage memory you're invoking undefined behavior. When you do this you may observe inconsistent and confusing results. In your case, adding the printf seemed to fix the problem. The reason that this affected the behavior of your code is implementation dependent and - once you've introduced undefined behavior - beyond your control.

edited Jul 8, 2012 at 22:27

answered Jul 8, 2012 at 18:59

pb2q

59.9k19 gold badges150 silver badges152 bronze badges

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

5 Comments

divinedragon Over a year ago

Thanks. It worked perfectly fine. Can you explain why was the segmentation fault without the printf() statement and with it it worked fine. Also, I think when we declare a pointer, it automatically assigned to NULL. Is that not true?

Dietrich Epp Over a year ago

@divinedragon: A pointer variable will have the value NULL at program start if and only if the variable is global (this includes static variables). Local variables take an "indeterminate value" instead. There is no reason it worked with printf, there is only coincidence.

phonetagger Over a year ago

@divinedragon - Several years ago I was amazed that an uninitialized pointer made it for years in a released product without ever causing a problem, until something else unrelated to that pointer was changed, exposing the uninitialized pointer bug. Out of dumbfounded curiosity, I dumped a disassembly of the old ("working") version of the code and traced backwards, discovering that the same stack location was used at one time for the local pointer variable, and at an earlier point (IN A DIFFERENT FUNCTION) as something else, which was always zero (i.e. "NULL") when exiting that function.

phonetagger Over a year ago

...and when the code was modified, those two entities (the uninitialized pointer and the other variable that was always zero upon exiting the earlier function) no longer lined up to the same stack location, exposing the bug.

Dennis Meng Over a year ago

@divinedragon What's happening is that your program is still wrong with the printf statement; you're just getting lucky. Just because it doesn't segfault doesn't mean the code is 100% correct. As people have implicitly pointed out, pointers do not get automatically assigned to NULL.

vanza · Accepted Answer · 2012-07-08 19:00:56Z

3

last and head are not being initialized. You can turn on compiler warnings that will help you out:

$ gcc -O1 -Wall w.c -o we
w.c: In function ‘main’:
w.c:30:23: warning: ‘last’ may be used uninitialized in this function [-Wuninitialized]
w.c:26:12: warning: ‘head’ may be used uninitialized in this function [-Wuninitialized]

(Note that gcc needs -O1 for -Wuninitialized to work.)

answered Jul 8, 2012 at 19:00

vanza

9,9132 gold badges33 silver badges35 bronze badges

1 Comment

divinedragon Over a year ago

Thanks. It worked perfectly fine. Can you explain why was the segmentation fault without the printf() statement and with it it worked fine. Also, I think when we declare a pointer, it automatically assigned to NULL. Is that not true?

Daniel Fischer · Accepted Answer · 2012-07-08 18:59:51Z

2

You're not initialising head and last,

NODE *head, *last, *newnode = NULL;

so they have whatever garbage bits are where they are allocated and if (head == NULL) can fail even though head is not properly set. Initialise them to NULL too.

answered Jul 8, 2012 at 18:59

Daniel Fischer

184k19 gold badges319 silver badges436 bronze badges

2 Comments

divinedragon Over a year ago

Thanks. It worked perfectly fine. Can you explain why was the segmentation fault without the printf() statement and with it it worked fine. Also, I think when we declare a pointer, it automatically assigned to NULL. Is that not true?

Daniel Fischer Over a year ago

Only static variables are initialised automatically. Your pointers are automatic variables, so they're not initialised. As for why printf averted the crash, I can't tell, it's undefined behaviour.

Collectives™ on Stack Overflow

Segmentation fault in C

3 Answers 3

5 Comments

1 Comment

2 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

5 Comments

1 Comment

2 Comments

Your Answer

Sign up or log in

Post as a guest

Related