4 ms·
Here's even more "tastier" 2 lines version: remove_list_entry(entry) { for (indirect = &head; (*indirect) != entry; indirect = &(*indirect)->next);
by sytelus 10y ago
Here's even more "tastier" 2 lines version:
remove_list_entry(entry)
{
for (indirect = &head; (*indirect) != entry; indirect = &(*indirect)->next);
*indirect = entry->next;
}
The big problem: both Linus's and above versions don't handle the case if entry wasn't found, for example, if entry was null. This is the fundamental issue with trying make code overly compact: sometime you lose the sight of important edge cases. Segfaults are not tasty. I generally prefer to write code that reflects my thought process and avoid unnecessarily try making it compact. As Knuth had said "Programs are meant to be read by humans and only incidentally for computers to execute”. Uglier versions allows to explicitly documents edge cases. This enables future maintainer to make sure these cases are covered when s/he makes code changes. Obviously taking this to another extreme would ruin this. The better taste lies somewhere between the compact Linus's version and some zeolite's too verbose version.
Small problem: Another thing to think about is extra pointer redirection required in Linus's code. This is fine for most cases but if I was dealing with very large list over and over then that's unnecessary perf hit.
- thomasahle 10y agoThe 'entry wasn't found' case is an entirely different issue than what Linus is showcasing. It may or may not be relevant for the particular function, depending on its contract and documentation.