C- Efficient code vs Good Code [closed]

Question

I am learning C, and so far have a decent understanding of the language, I recently implemented a singly linked list which is pretty simple. I had watched a Ted Talks with Linus Torvalds, he had mentioned good code vs bad code and provided these two examples of removing an entry from a linked list:

/*bad code*/
remove_list_entry(entry){
    prev = NULL;
    walk = head;

    while(walk != entry){
        prev = walk;
        walk = walk->next;
    }

    if(!prev)
         head = entry->next;
    else
         prev->next = entry->next;


/*good code*/
remove_list_entry(entry){
    indirect = &head;

    while ((*indirect) != entry)
        indirect = &(*indirect)->next;

    *indirect = entry->next;
}

It seems to me that his example of good code is efficient, but I wouldn't have figured this out myself. Is there any recommendations for good practice while programming in C that I can follow? Or certain ways to manipulated pointers as he did in his example? As well as bad habits and practices to avoid while programming in C.I hope this isn't too broad of a question.

Answer 1

Here's my short answer: you're not wrong, but neither is Linus. I am an experienced programmer with a lot of work in C and I find his version of this linked list code basically unreadable. I would never write this code myself.

He used this example not so much to show OK vs. not-OK, but to demonstrate how clearer thinking about a problem can lead to solutions that are more compact and have fewer edge cases (and conditional flows) to consider and test. On that score, his is an improvement.

"Readability" is typically cited as a first-order consideration in modern programming, so that both you and others can understand quickly what code is trying to achieve. This is traded off against complexity and performance considerations, but it's up to you to make that trade-off.

Answer 2

The particular example from Linus demonstrates how to avoid branches by using indirection. The two separate code paths represented by head = entry->next and prev->next = entry->next can be united into one by using a pointer to the thing that is being assigned to: *indirect = entry->next.

This approach can lead to very elegant, linear code with fewer branches.

The general pattern is that you can turn this code:

int a = 0, b = 0;

if (some_condition) {
    a = 1;
} else if (other_condition) {
    b = 1;
}

/* continue working with "a" and "b" */

Into:

int a = 0, b = 0;
int *p;

if (some_condition) {
    p = &a;
} else if (other_condition) {
    p = &b;
} else {
    p = NULL;
}

/* consequently, work only with "*p" and never again touch "a" or "b" */
if (p) {
    *p = 1;
}