Bob Hofstede
Bob Hofstede

Reputation: 27

Remove a node at the tail of a linked list C

I wrote code to remove the node at the tail of a linked list. The code works properly in different testcases, but I think I made my code a bit cumbersome. However, I don't see what I can do differently?

node_t *remove_t (node_t *l){
if (l==NULL){
    return l;
}
else {
    node_t *curr=l;
    node_t *ret=l;
    if (curr->next==NULL){
        l=NULL;
        return l;
    }
    else {
        while (curr->next->next!=NULL){
            curr=curr->next;
        }
        curr->next=NULL;
        free(curr->next);
        return ret;
    }
}
}

Upvotes: 0

Views: 507

Answers (3)

Brad Pitt
Brad Pitt

Reputation: 416

You didn't seem to free the tail node.

curr->next=NULL;
free(curr->next);

You won't be able to free curr->next if you already make it NULL.

My implementation:

void remove_tail(node_t *l) {
    if (l == NULL) return;
    if (l->next == NULL) {
        free(l);
        l = NULL;
        return;
    }
    node_t *prev = l;
    node_t *curr = l->next;
    while (curr->next != NULL) {
        prev = curr;
        curr = curr->next;
    }
    prev->next = NULL;
    free(curr);
}

Upvotes: 0

David C. Rankin
David C. Rankin

Reputation: 84561

It is much, much easier if you keep a pointer-to-pointer to node, then iterate to the end of list and simply free the last pointer and set it NULL, e.g.

/** delete last node */
void del_last_node (node_t **l)
{
    node_t **ppn = l;       /* pointer to pointer */
    node_t *pn = *l;        /* pointer to node */

    for (; pn->next; ppn = &pn->next, pn = pn->next) { } /* iterate to last */

    free (*ppn);           /* free node */
    *ppn = NULL;           /* set pointer NULL */
}

Upvotes: 1

dash-o
dash-o

Reputation: 14452

I'm not sure that you can change the logic much - as your approach of 3 different cases (empty list, list with 1 item, and list with >1 items) is reasonable. You can format the code for easier reading: Something like:

node_t *remove_t (node_t *l){
    // case 1: Empty list
    if (l==NULL){
        return l;
    } ;

    // case 2: List with one item. Return empty list.
    node_t *curr=l;
    if (curr->next==NULL){
        // Remember to free this element.
        free(curr) ;
        l=NULL;
        return l;
    } ;

    // case 3: list > one item
    // Move curr to last item
    while (curr->next->next!=NULL){
        curr=curr->next;
    }
    // Note order of free/null setting.
    free(curr->next);
    curr->next=NULL;

    return l;
}

Upvotes: 0

Related Questions