Horror Code: Paid By The Number Of Rows

The first thing I've thought is: "I'm missing something". But after few seconds the doubt that the author of the following code is paid for the number of lines of code come to my mind. Judge yourself:

struct data_node *node = calloc(1, sizeof(*node));  
if (node) {  
        node->data = NULL;
        node->info = NULL;
        node->next = NULL;
}
comm->data_list = node;  

Since node isn't used anywhere else, those six lines are exactly equivalent to the following:

comm->data_list = calloc(1, sizeof(*(comm->data_list)));  

The function calloc returns a pointer to a memory area that is guarantee to be blank, so there is non need to assign NULL to the members of the structure.

Checking the value returned by calloc is a good thing. It may be that there is no more memory available so that error should be handled. But in this case the author didn't take any action in case of failure.

Use a temporary variable may be useful in some situations, but this is not one of those.

In conclusion, the code is formally correct: it doesn't have bugs or cause harms and maybe the compiler can optimize it. But the point is that redundant and useless code can lead to difficulties in reading and understanding besides subtle hard-to-find bugs.


Image by goopy mart taken from Flickr licensed under the Creative Commons Attribution-NonCommercial-ShareAlike 2.0 Generic license.

Luca Sommacal

Luca Sommacal

Italian developer (mainly in C for embedded platforms), Linux learner, addicted to rock music, history, science and few other things. Follow me on Twitter

comments powered by Disqus