# Delete last node in singly linked list

In a TED talk Linus Torvalds made a point about “good taste” at approximately 14:10 in the interview. I read through his examples of “bad taste” and “good taste” and wanted to implement the same principle for a function that deletes the last node in a singly linked list. I also followed his coding style.

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

/* linked list structure */
struct node {
int data;
struct node *next;
};

/* create new node */
struct node *new_node(int data, struct node *next)
{
struct node *new = malloc(sizeof *new);
if (!new) {
fprintf(stderr, "Error: memory allocation failed\n");
exit(EXIT_FAILURE);
}
new->data = data;
new->next = next;
return new;
}

/* delete last node */
void delete_last(struct node *head)
{
if (head == NULL) {
fprintf(stderr, "Error: linked list underflow\n");
exit(EXIT_FAILURE);
}
struct node **cursor = &head;
while ((*cursor)->next != NULL)
cursor = &(*cursor)->next;
free(*cursor);
*cursor = NULL;
}

• It's generally not good taste to exit() from a minor function call.
– Mr R
Apr 5 '21 at 20:38
• @MrR Alright, what's the alternative? Apr 5 '21 at 21:39
• Instead of void make it int (or if you like bools, add the stdbool header) and return something to indicate the failure to the calling function. Apr 6 '21 at 4:15
• As @debdutdeb said - have an error code, or in the case of delete (if there is no list the after effect is the same - there is still no list / there is 1 least node on the end - so it's not so much an error). AND of course the new_node method either needs documentation or is a new_element_before because it's not inserting at a random point it's inserting in a way that would break an existing list (if you put it say before the last element) - you'd end up with a final node pointed to by two nodes.
– Mr R
Apr 6 '21 at 20:32
• Lots of good advice over here codereview.stackexchange.com/questions/136077/… - it's C++ but is more C like than not.
– Mr R
Apr 6 '21 at 20:35

## 2 Answers

It's quite presumptive for a function to exit the program like this:

    if (!new) {
fprintf(stderr, "Error: memory allocation failed\n");
exit(EXIT_FAILURE);
}


I'd argue that the caller is also better positioned to know whether an error message is useful, too:

    if (!new) {
return new;
}


Similarly, removing from empty list is better conveyed by a return value.

Explicit comparison against NULL seems clunky to me (and inconsistent with the test of new above), given that pointers have a well-understood truthiness:

if (!head) {

while ((*cursor)->next)


I also followed [Torvalds'] coding style

I'm all for coding style standards, though I decline to listen to all of his suggestions simply because he's the loudest (seriously, very very loud) voice in the room. His blind obedience to 1988's K&R with no other rationale I find short-sighted as well. Anyway, enough editorializing:

Your non-parenthesized sizeof *new falls on a matter in his guide that is a little ambiguous -

The notable exceptions are sizeof, typeof, alignof, and __attribute__, which look somewhat like functions (and are usually used with parentheses in Linux, although they are not required in the language)

Whereas parens are not required after sizeof, I see them in the majority of code I encounter so I'd recommend sticking with them.

I warn against writing keywords from C++ as symbol names in C, in this case new. If ever you want this to be readily C++-compatible, this will cause you grief.

• I differ here, and don't like unnecessary parens when using sizeof - I find that helps draw attention to sizeof (type) as opposed to sizeof expr. As you say, it's all personal preference, and being consistent is what matters most. Apr 8 '21 at 8:51