C programming segmentation fault linked list program

Question

I am fairly new to c (and this site) and I am having a lot of issues with segmentation faults. I am writing a program that creates a linked list of numbers and inserts values in ascending order.

     void insert(struct element **head, struct element *new){   
            if((*head)->next == NULL && (*new).i > (*(*head)->next).i){
                (*head)->next = new;
                return;     
            }
            if((*head)->next == NULL && (*new).i < (*(*head)->next).i){
                new->next = (*head)->next;
                *head = new;    
                return;
            }
            struct element *prev = *head;
            struct element *current = (*head)->next;
            while(current->next != NULL){
                if((*new).i < (*current).i){
                    prev = current;
                    current = current->next;
                } else if((*new).i > (*current).i){
                    new->next = current;
                    prev->next = new;
                }
            }
        }
        int main (void){
            struct element **head;
            int value;
            printf("%s", "TEST" );
            printf("%s" , "Please type in an integer value. ");
            scanf("%d" , &value);
            printf("%s", "TEST" );
            do{
                printf("%s", "TEST" );
                struct element *new;
                if((new = malloc(sizeof(struct element))) == NULL){
                return(NULL);
                }
                printf("%s", "TEST" );
                (*new).i = value;
                printf("%s", "TEST" );
                if(head == NULL){
                    *head = new;
                    printList(*head);
                }  else if(value <= 0){
                    printListBackwards(*head);
                }   
                else {

                    insert(head, new);
                    printList(*head);
                }
                } while(value > 0);

I don't need help on whether the logic is correct with inserting or anything. I haven't even had a chance to really test it because I immediately get a segmentation fault after I enter an integer after the prompt. I know it seems funky, but the specs call for you to use a pointer to a pointer to a structure (the head of the linked list).

When sharing code, make sure that other users can quickly compile your code and reproduce the error. Your code is missing structure definitions, functions and even a closing brace in the main function. Also, before asking for help, make sure that your code doesn't show warnings/errors when compiling. Try compiling with the flags -Wall -Wextra. When figuring out segfaults I find valgrind an extremely useful tool, give it a try. Just remember to compile with the -g flag to generate debugging symbols — Cristiano Sousa
– Cristiano Sousa, Commented Apr 5, 2013 at 1:53

Parker Kemp · Accepted Answer · 2013-04-05 01:55:20Z

2

Are you sure you want head to be an element** and not an element*? That extra degree of separation is causing you problems, not the least of which is very difficult-to-read code.

Here's the main thing that jumps out at me:

if(head == NULL){
    *head = new;
    printList(*head);
}

You're confirming that head is a NULL pointer, and then immediately trying to dereference it with *. If you really insist on head being a double-pointer, then you need to dynamically allocate it before dereferencing it. Like so:

if(head == NULL){
    head = malloc(sizeof(element*));
    *head = new;
    printList(*head);
}

That actually may not be syntactically perfect (I come from C++), but you get the idea. Speaking of C++ though, it's usually considered bad practice to name a variable "new" in C, because new is a keyword in C++.

answered Apr 5, 2013 at 1:55

Parker Kemp

7851 gold badge10 silver badges25 bronze badges

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

3 Comments

user2247284 Over a year ago

Thank you. I am going to change head to a single pointer and see if it works better. I am still unsure of why when I run the program, it does not print out the second "TEST" line from the main before it fails.

Jim Balter Over a year ago

@user2247284 Your printf strings don't end with newlines so they aren't getting flushed. Also, It's common to just do printf("TEST\n");

Parker Kemp Over a year ago

To expand on what Jim said, printf doesn't actually print immediately unless it has to. In your program, it put "TEST" in the buffer, but then decided to wait since it hadn't reached a newline yet. Then your program crashed before it ever got around to flushing the buffer.

Jim Balter · Accepted Answer · 2013-04-05 02:06:13Z

0

struct element **head;

You don't want that. Instead,

struct element *head = NULL;

Then, when you call insert, use

insert(&head, new);

You have many other bugs and poor usages, but that's a start for your specific problem.

answered Apr 5, 2013 at 2:06

Jim Balter

16.5k3 gold badges46 silver badges70 bronze badges

Comments

nonsensickle · Accepted Answer · 2013-04-05 02:08:43Z

0

There is a seg fault being caused on the second line of your post

if((*head)->next == NULL && (*new).i > (*(*head)->next).i){
    (*head)->next = new;
    return;     
}

Segmentation fault means that you are trying to access memory that you are not allowed to. For example, you can't dereference a NULL pointer.

Your if statement is evaluated like this. Check that (*head)->next is null.

if it is not NULL, then skip the rest.

if it IS NULL, then you can replace each following (*head)->next with NULL. This means that the following part && (*new).i > (*(*head)->next.i) can be rewritten as follows && (*new).i > ((*NULL).i)...

In short, you are trying to dereference a NULL pointer value.

Please also refer to @Parker Kemp's post. There is a number of times that you are checking for NULL correctly but misinterpreting it's meaning.

I could rewrite the code for you but I think that you will benefit more from going through a tutorial like this one or this one

I strongly recommend drawing a diagram of your data structure and drawing arrows for pointers.

edited Apr 5, 2013 at 2:08

answered Apr 5, 2013 at 1:45

nonsensickle

4,5682 gold badges37 silver badges64 bronze badges

2 Comments

user2247284 Over a year ago

Ok thanks. Could you tell me why though that when I run the program, it fails to output the second "TEST" line before it even attempts to call the function insert? And what is a safe way to check for NULLs in C?

nonsensickle Over a year ago

You are correctly testing for NULL but you're not using the correct comparison. if (pointer == NULL) ... will tell you if the pointer contains a NULL value meaning that it points to forbidden memory.

Collectives™ on Stack Overflow

C programming segmentation fault linked list program

3 Answers 3

3 Comments

Comments

2 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

3 Comments

Comments

2 Comments

Your Answer

Sign up or log in

Post as a guest

Related