deleting last element from linked list

0
votes

For a project I need a linked list implementation in C which enables me to delete the last element.

However, I don't know how to achieve this. The idea was to create a function (deleteLast) which iterates over the list until the next of the next element is NULL (so until the second last element is reached) to then free the reference to the last element.

However, I get an error "expression must have pointer-to-struct-or-union type" when trying to compile.

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

struct cell{
    int x_coord,y_coord;
    struct cell *next;
} cell;

struct cell AddToList (struct cell *list, int x,int y);
int listlength(struct cell * list);
void deleteLast(struct cell **list);

struct cell AddToList(struct cell *list,int x,int y){
    struct cell *new_cell;
    new_cell = malloc(sizeof(struct cell));
    new_cell->x_coord=x;
    new_cell->y_coord=y;
    printf("Added new coordinates %i %i",x,y);
}

int listlength(struct cell *list){
    int i=0;
    while(list->next != NULL){
        i++;
    }
    return i;
}

//takes a pointer as reference, because in C parameters are given as values
//see: https://stackoverflow.com/a/35021864
//calls should look like deleteLast( &list )
void deleteLast(struct cell **list){
    struct cell *currentcell = *list;
    while(*list->next->next != NULL){ //expression must have pointer-to-struct-or-union type
        //free list->next
    }
}

Where's the error?

2
cpointerslinked-list
Your compiler will be telling you where the error is. Perhaps the malloc needs a cast? (struct cell *)malloc – noelicus
@noelicus I meant the deleteLast method. Take a look at the comment at the while loop, that's the errormessage I get. – Daniel Siegel
Your error is that *list->next->next means *(list->next->next) and not (*list)->next->next. – user6556709
Probably you got segmentation fault, you should cast the void pointer returned by the malloc() --- ' new_cell =(struct cell *) malloc(sizeof(struct cell)); ' – Mr. Suklav Ghosh

2 Answers

1
votes
void deleteLast(struct cell **list){
    struct cell * currentcell = *list;
    while(currentcell->next->next != NULL) {
        currentcell = currentcell->next;
    }
    free(currentcell->next);
}
1
votes

Since Daniel Siegel explicitly asked me to extent the answer of Andrew St. Pierre. Here is what I wrote in the comments.

You always have to check if your pointer is not a NULL before you dereference it and you have to assign NULL explicit after freeing something cause free will not do that for you but leave you with a pointer which looks good but points to nothing.

void deleteLast(struct cell **list){
    //Do nothing if list is a NULL
    if (!list){
        return;
    }
    struct cell * currentcell = *list;
    //Do nothing if list is empty
    if (!currentcell){
        return;
    }
    //Check if first element is last element
    if (!currentcell->next){
        free(currentcell);
        //assign NULL to the pointer in list not to currentcell
        //since currentcell is a local variable.
        *list = NULL;
        return;
    }
    while(currentcell->next->next != NULL) {
        currentcell = currentcell->next;
    }
    free(currentcell->next);
    //explicit set last element to NULL
    currentcell->next = NULL;
}