user2247284
user2247284

Reputation: 1

C programming segmentation fault linked list program

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).

Upvotes: 0

Views: 874

Answers (3)

nonsensickle
nonsensickle

Reputation: 4528

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.

Upvotes: 0

Jim Balter
Jim Balter

Reputation: 16406

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.

Upvotes: 0

Parker Kemp
Parker Kemp

Reputation: 765

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++.

Upvotes: 2

Related Questions