Segmentation fault in Linked list /C

Question

I wanna remove the last item from my linkedlist, but become an Segmentation-fault. So some pointer is wrong. Here is my code.

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

struct listenElement{
    int wert;
    struct listenElement *next;
};
static  struct listenElement* anfang = NULL;
int removeElement(void) {
    if (anfang == NULL){     
        return -1;
    }
    else{
        int l = NULL;
        struct listenElement *p = anfang->next ;     
        if (p->next == NULL){
            l = p->wert;
            free(p);
            anfang->next = NULL;
        }
        else
        {
            while(p->next != NULL){
              anfang = p;
              p = p->next;
              l = p->wert;
              free(p);
            }
            anfang =p;
            anfang->next= NULL;
        }
        return l;
    }
}

You should always format your code before letting others see it. — kviiri
– kviiri, Commented Apr 27, 2014 at 10:18
Your while loop is wrong, it frees p, but then you access it with p->next or anfang=p; anfang->next=NULL;. — nullptr
– nullptr, Commented Apr 27, 2014 at 10:21

Grijesh Chauhan · Accepted Answer · 2014-04-27 10:42:27Z

2

Note once you have called free on some pointer you can't access that pointer (memory via other pointer also), doing this calls undefined behavior—it is an illegal memory instruction that can be detected at runtime that may causes — a segmentation fault as well:

Following code is wrong:

while(p->next!=NULL){// <--+  you use `p` - that is invalid 
    anfang = p;    //      |
    p = p->next;   //      |
    l = p->wert;   //      |
    free(p);//-------------+  you free `p`
}

Additionally learn Indenting C Programs.

There are some more mistakes in your code:

Logic is incorrect, you don't consider the case when there is only one node present. You need to add,

 // this first pice of code to add
 // Case-1: when nodes in linked list are = 1
if (anfang->next == NULL){
   free(anfang);
   anfang = NULL; reset to NULL   
   return 1; // as you returns 
 }

You while loop can be improved and the use of anfang in loop and after loop is wrong. anfang should point to first node always, you just need to remove last node. And if there are more than one node in linked list then anfang value doesn't change (read comments):

 // this code should be after above code
 // Case-2: when nodes in linked list are > 1
 p = anfang;  // 
 last = anfang->next; // last node
 // a loop for travel to reach - last node in linked list
 while(last->next != NULL){//stop when last - points to last node
    p = last;      // every time before update last make `p` the `last`
    last = last->next;
 }
 p->next = NULL; // make `p` as last node, to remove last node
 free(last);  // free memory for last node

Logically you where wrong, In your code you call free within while loop whereas your objective is to remove just one—the last node.

edited Apr 27, 2014 at 10:42

answered Apr 27, 2014 at 10:20

Grijesh Chauhan

58.6k20 gold badges145 silver badges214 bronze badges

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

5 Comments

kviiri Over a year ago

A very good answer. I also think the use of the variable anfang seems wrong, you might want to include something about it too.

Grijesh Chauhan Over a year ago

@kviiri I direct target at seg-fault, there are more mistakes also I add in my answer thanks

Grijesh Chauhan Over a year ago

Did you try to understand my answer? Did you write rest of the code yourself? Anyways complete removeElement(void) function should look like this click here

Grijesh Chauhan Over a year ago

@user3578012 You can "add comment", check this link

Grijesh Chauhan Over a year ago

@user3578012 You should add declarations for last and p inside function definition as I added in the linked code.

Collectives™ on Stack Overflow

Segmentation fault in Linked list /C

1 Answer 1

5 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

5 Comments

Your Answer

Sign up or log in

Post as a guest

Related