CODEWITHSUNDEEP
clistpointerslinked-list
0xAX
0xAX

Reputation: 21817

Get the last element error in a C double linked list

I try to write a double linked list in C. And now I write a getLast element function:

Dlist* getLast(Dlist **list)
{
    if (list != NULL)
    {
        while((*list) != NULL)
            (*list) = (*list)->next;
    }
    return (*list);
}

I get a segmentation fault:

Program received signal SIGSEGV, Segmentation fault. 0x080485ce in getLast (list=0x804a008) at src/dlist.c:29 29 (*list) = (*list)->next;

I add one element, ans it's OK. When I try to add a second element, I get a segmentation fault.

I call this function so:

Dlist* addItemAtStart(Dlist** list, Pair* value)
{
    Dlist* last = NULL;
    last = getLast (*list);
    ...
}

What's wrong?

Upvotes: 0

Views: 470

Answers (5)

ArgyrisSofroniou
ArgyrisSofroniou

Reputation: 122

Next time you have an issue in your code, give all your code, not just a part of it!

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

struct node{
    void* value;
    struct node *prev, *next;
};
/*This Function is Unnecessary
struct node * createList(){
    struct node *newNode=malloc(sizeof(struct node));
    newNode->value = NULL;
    newNode->next = NULL;
    newNode->prev = NULL;
          
    return newNode;
}
*/
/*
 *This Function is also Unnecessary
 *Get last element from Dlist
struct node* getLast(struct node* node){
    while(node){
        if (node->next==NULL) break;
        node=node->next;
    }
    
    return node; 
}
*/
/*add element to list at start*/
void addItemAtStart(struct node **head, void *value){
    struct node *newNode=malloc(sizeof(struct node));
    
    if (newNode==NULL) return;
    
    newNode->value=value;
    newNode->prev=NULL;
    newNode->next=*head;
    
    if (*head!=NULL) (*head)->prev=newNode;
    *head=newNode;
}

int main(){
    struct node *list=NULL;
    
    addItemAtStart(&list, "apple");
    addItemAtStart(&list, "lemon");
    addItemAtStart(&list, "orange");
    addItemAtStart(&list, "peach");
    
    return 0;
}

Upvotes: 0

Platinum Azure
Platinum Azure

Reputation: 46183

You need to store the list pointer in a temporary variable so you don't clobber your list (or other memory):

Dlist* getLast(Dlist **list)
{
  if (list != NULL)
  {
      Dlist *ptr = *list;
      if (ptr == NULL)
          return NULL;

      while(ptr->next != NULL)
          ptr = ptr->next;

      return ptr;
  }
  return NULL;
}

Upvotes: 1

Perception
Perception

Reputation: 80603

You are clobbering all your list pointers. Many of your problems are rooted in not accepting the basic list structure. A list is its first element - you do not need represent it as a pointer to that first element.

Some code to illustrate?

DIList * myList ; // This is your list, add elements to it

DIList * lastElement = getLast(myList); // Last element in your list, also a list

DIList * getLast(DIList * aList) {
    if(aList == NULL) return NULL;

    DIList * aNode = aList;
    while(aNode->next != NULL) aNode = aNode->next;

    return aNode;
}

Upvotes: 1

Santanu
Santanu

Reputation: 11

In addItemAtStart, why are you using:

    last = getLast (*list);

instead of:

    last = getLast(list);

Also, in getLast, shouldn't you be using:

    while((*list)->next != NULL)

instead of:

    while((*list) != NULL)

Upvotes: 0

Karoly Horvath
Karoly Horvath

Reputation: 96258

Your code returns a NULL pointer.

while(*list->next != NULL)

Upvotes: 3

Related Questions