[rdtsc]

Redis is my favorite example of a very clean and beautiful example of C code, just the right amount of comments, good variable names. It is great work. You can tell Salvatore cares and has passion for what he does just by looking at his work.

https://github.com/antirez/redis/tree/unstable/src

[free652]

Wow it's awesome, comparing to the most of the C code that I saw it's beautiful. I like the long method names, because I can actually understand what they are doing.

[matthiasv]

Maybe you haven't looked very often but nice code is out there for quite some time. Take a look at anything related to the GNOME stack (GLib, GTK+, all the applications) or the Linux kernel for example. And quite frankly, I stumble upon badly written Python way more often than C.

[spullara]

The code for those might be great, but Redis is beautiful inside and outside. I'm not a fan of the actual output of the GNOME stack. I agree the Linux kernel code is generally great to read and to use.

[avtar]

"And quite frankly, I stumble upon badly written Python way more often than C."

I'm curious, which public codebases would you consider to be examples of well written Python?

[matthiasv]

The often cited requests library, flask and Django. Some counter examples are high profile projects such as IPython and Matplotlib.

[avtar]

Thanks. I'll spend some time with Requests over the weekend. It looks like the person who maintains that library is also behind this http://docs.python-guide.org/en/latest/#writing-great-code

[16s]

IMO, OpenBSD and tarsnap are other notable samples of clean C code.

[maurycy]

I'd also mention FreeBSD kernel and Niels Provos' code.

[Someone]

What are you comparing that with? I find it fairly normal for C code dating from past, say 2000.

Also, one thing that made a negative impression on me is https://github.com/antirez/redis/blob/unstable/src/adlist.c:

  /* Free the whole list.
   *
   * This function can't fail. */
  void listRelease(list *list)
  {
      unsigned long len;
      listNode *current, *next;

      current = list->head;
      ...

If you write such a comment, you better make sure it is true. And no, an early "list == NULL" check is not the only thing missing.

[rdtsc]

Compared to an average of other projects coded in C out there.

Others mentioned kernel, GNOME. Redis is nice because it is fairly self contained. It has a good set of networking code, some command parsing, storage. Kind of a happy medium.

Anyway, I didn't look in depth at understanding the semantics of every module.

Here are some more things (besides NULL check I can see that are worrisome in that particular piece of code):

* can len be out of sync with actual list length

* can list->free be defined (non null) but value wasn't allocated with an malloc perhaps

* what if an element is inserted twice in list, will list->free be called on already free-ed area

* on 64 bit machine unsigned long will be 64 bit and on 32 bit it will be 32 bit, is that problem?

Which ones do you see?

Now those are issues I see by looking at the module in isolation. But it is not quite an isolated. It is part of a large module. Sometimes there are invariants that are enforced at the input (at the system boundary) and so it is not necessary to always keep checking for NULL or validating inputs in every single internal function.

That is one good lesson I learned from Erlang. Check your inputs at the boundary then code for the "happy" path and let error result in quick and early failure. Maybe prefer a quick segfault rather than a dangling pointer or wondering later how exactly to handle NULL pointer if it is not really expected to be NULL.

[etimberg]

you'd probably want to zero the length in the list structure and remove the dangling pointer to the first element otherwise you run the risk of a double-free.