Create linked list causes segmentation fault

Question

I am attempting to overcome my fear of linked lists since my confidence built after my last few posts. I have re-attempted an older algorithmic exercise I had done before (incompleted.) Now, I am confident it is complete, however, I'm thrown the usual segmentation fault error. I wonder, if there is a way to understand what causes this issue in the code, I can rarely identify it (but I'm still novice.)

For example:

#include <stdio.h>
#include <stdlib.h>

typedef struct node {
    int DATA;
    struct node* LINK;
} node;

void create(struct node* head, int item){
    struct node* new;
    new = (struct node*)malloc(sizeof(new));
    if (new == NULL){
        printf("Memory not Available");
    }
    new->DATA=item;
    new->LINK = NULL;
    if (head == NULL){
        head = new;
    }else {
        struct node* temp;
        temp = head;
        while(temp->LINK != NULL){
            temp = temp->LINK;
        }
        temp->LINK=new;
    }
}

int main(){

    node* former = NULL;
    int itm = 7;
    create(former, itm);

    printf("%i", former->DATA);

    return 0;
}

Will produce the segmentation fault.

The algorithm I am attempting:

The following algorithm creates a node and appends it at the end of the existing list. head is a pointer which holds the address of the header of the linked list and item is the value of the new node. NEW is a pointer which holds the address of the new node and temp is a temporary pointer.

Algorithm: CREATE (HEAD, ITEM)
1. [Create NEW node]
a) Allocate memory for NEW node.
b) IF NEW = NULL then Print: “Memory not Available” and Return
c) Set NEW→DATA = ITEM
d) Set NEW→LINK = NULL
2. [Whether List is empty, head is the content of HEADER]
If HEAD = NULL then Set HEAD = NEW
3. Else
a) Set Temp = HEAD
b) While Temp→LINK ≠ NULL do
Set Temp = Temp→LINK
[End of while]
c) Set Temp→LINK = NEW
[End of IF]
4. Return

Based on the suggested link I continue to get the same error:

void create(node** head, int item){
    *head = NULL;
    node* new;
    new = (node*)malloc(sizeof(*new));
    if (new == NULL){printf("Memory not Available");}
    new->DATA=item;
    new->LINK = NULL;
    if (head == NULL){head = &new;}else {
         node* temp;
        temp = *head;
        while(temp->LINK != NULL){
            temp = temp->LINK;}
        temp->LINK=new;}}

int main(){
    node* former;
    int itm = 7;
    create(&former, itm);
    printf("%i", former->DATA);
    return 0;
}

You defined a typedef, why do you keep writing `struct node *` instead of `nodes *`? — Barmar, Dec 29 '22 at 20:24
`former` is a null pointer. `create()` assigns the local `head` variable, it doesn't assign the caller's variable. — Barmar, Dec 29 '22 at 20:26
`sizeof(new)` is the size of the ***pointer*** `new` which is incorrect. It needs to be `sizeof(*new)`. — Some programmer dude, Dec 29 '22 at 20:27
Another problem is that in C arguments are passed *by value*. That means the value in the call is *copied* into the functions *local* argument variable. All changes to that variable will be lost when the function returns, as the original value won't be affected. — Some programmer dude, Dec 29 '22 at 20:28
@Barmar Thank you for sharing, I had not realised this option. — Emil11, Dec 29 '22 at 20:42
Hi @Someprogrammerdude, I made the amendment in my script however, the error continues to persist. I'm likely messing up with a pointer somewhere? — Emil11, Dec 29 '22 at 20:43
@Someprogrammerdude That's not "another" problem, that's the same problem I mentioned in my second comment. The linked duplilcate question explains how to resolve it. — Barmar, Dec 29 '22 at 20:47
@Emil11 What option are you talking about, using `node*`? What did you think the purpose of `typedef` was? — Barmar, Dec 29 '22 at 20:48
@Barmar I have checked the linked duplicate and cannot manage to resolve when using a pointer. If I make `former` not a pointer, it works but I cannot use `->` and have to use `.` instead. But this only returns the result of a memory address, as opposed to the value stored. I may need an example based on my set-up to understand correctly :/ — Emil11, Dec 29 '22 at 20:54
@Barmar BTW I have attempted the answer from the question and it continues to produce a `segmentation fault` error. I have updated my post to show what I changed. — Emil11, Dec 29 '22 at 20:59
Of course it should be a pointer. You need to change `head` to `node**`, and call the function as `create(&former, item)`. And inside the function, you have to change all uses of `head` to `(*head)`. — Barmar, Dec 29 '22 at 21:00
A simpler solution would be for `create` to return the new node. Then you would use `former = create(former, item);` — Barmar, Dec 29 '22 at 21:01
@Barmar I will attempt the 'simpler' solution, however, your former method according to the linked article is not working. See my update. — Emil11, Dec 29 '22 at 21:10
`if (head == NULL){head = &new;}` should be `if (*head == NULL) { *head = new; }` — Barmar, Dec 29 '22 at 21:11
Why do you need that `if` statement? The first line of the function is `*head = NULL;`, so the condition will always be true. You probably shouldn't do `*head = NULL;` at the beginning, so you can updating an existing node from the caller. — Barmar, Dec 29 '22 at 21:12
@Barmar btw the 'simpler' solution is working in my end, I actually usually implement things this way. Using `void` was a new approach based on what I had seen online. Thnak you. I'll attempt your latest comment also. — Emil11, Dec 29 '22 at 21:13
@Barmar Brilliant! that worked too, not sure why I missed that ... — Emil11, Dec 29 '22 at 21:15

Create linked list causes segmentation fault

0 Answers0