Live data from Hacker News

A quick look at the Redis source code

heychinaski.com

21–30 of 34 posts

Re: A quick look at the Redis source code

#21
post #11

Earlier quoted context omitted.

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.

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.

"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?

Re: A quick look at the Redis source code

#22
post #14

Nice post. I've been working quite a lot with the internals of Redis in the past few months. Adding custom commands along with the usual skimming through the builtins. Perhaps I should give people some insight by creating a few blog post as well. It's a really nice piece of software and written in clean, high quality, C. Not sure about the tests in Tcl though :).

I was quite surprised to see the tcl tests. I'm reserving judgement until I've tried writing one though.

Re: A quick look at the Redis source code

#23
post #14

Nice post. I've been working quite a lot with the internals of Redis in the past few months. Adding custom commands along with the usual skimming through the builtins. Perhaps I should give people some insight by creating a few blog post as well. It's a really nice piece of software and written in clean, high quality, C. Not sure about the tests in Tcl though :).

I was quite surprised to see the tcl tests. I'm reserving judgement until I've tried writing one though.

To be fair, the tests in themselves are alright, but I'm not to familiar with tcl and have had problems with running them in a CI build with a lot of redis-servers being left behind. As the test are as far as I've seen basically integration tests it would be quite nice to have them in something like python to make them a bit more easy to handle.

Re: A quick look at the Redis source code

#25
post #8

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

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.

Re: A quick look at the Redis source code

#26
post #21

Earlier quoted context omitted.

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.

"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?

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

Re: A quick look at the Redis source code

#27
post #25
post #8

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

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…

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.

Re: A quick look at the Redis source code

#28
post #25
post #8

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

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…

[deleted]

Re: A quick look at the Redis source code

#29
I'm certainly novice in C, but as I was reading, I wondered about this

  {"get",getCommand,2,"r",0,NULL,1,1,1,0,0},

  "The fourth field, set to "r", is specifying that the 
   command is read only and doesn’t modify any keys’ value 
   or state.  There are a whole bunch of one letter flags 
   that you can specify in this string that are explained 
   in detail in the nearby block comment.  The field 
   following this string  should always be set to zero, and 
   will be computed later.  It’s simply a bitmask 
   representation of the information implied by the string."
Why would you opt for this, when you could specify some constants and bitwise or them together? Isn't that a more common thing to do, than to calculate a bitwise flags at run time?

   COMMAND_READONLY | COMMAND_RANDOM | COMMAND_NOSIDEEFFECTS
ect ect ect.

I'm sure there's a good reason, but this style seems strange to me.

Maybe redis makes use of the string later? but I can't help but feel it should build the string based on the flags, rather than build the flags based on the string.

Re: A quick look at the Redis source code

#30

I'm certainly novice in C, but as I was reading, I wondered about this {"get",getCommand,2,"r",0,NULL,1,1,1,0,0}, "The fourth field, set to "r", is specifying that the command is read only and doesn’t modify any keys’ value or state. There are a whole bunch of one letter flags that you can specify in this string that are explained in detail in the nearby block comment. The field following this string should always be…

In defence of the technique, the command table is quite succinct and arguably more readable at a glance than if there were a bunch of constants |ed together. I have no idea whether this was the original motivation though.
Post reply on HN