1
\$\begingroup\$

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;
}
\$\endgroup\$
5
  • 2
    \$\begingroup\$ It's generally not good taste to exit() from a minor function call. \$\endgroup\$
    – Mr R
    Apr 5 '21 at 20:38
  • \$\begingroup\$ @MrR Alright, what's the alternative? \$\endgroup\$ Apr 5 '21 at 21:39
  • 1
    \$\begingroup\$ 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. \$\endgroup\$
    – debdutdeb
    Apr 6 '21 at 4:15
  • \$\begingroup\$ 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. \$\endgroup\$
    – Mr R
    Apr 6 '21 at 20:32
  • \$\begingroup\$ Lots of good advice over here codereview.stackexchange.com/questions/136077/… - it's C++ but is more C like than not. \$\endgroup\$
    – Mr R
    Apr 6 '21 at 20:35
1
\$\begingroup\$

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)
\$\endgroup\$
2
\$\begingroup\$

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.

\$\endgroup\$
1
  • \$\begingroup\$ 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. \$\endgroup\$ Apr 8 '21 at 8:51

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.