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;

How to Fix it

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 guaranteed 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 it can be greately optimized by the compiler. But the point is that redundant and useless lines of code can lead to difficulties in reading and understanding besides subtle hard-to-find bugs.

Image by goopy mart from Flickr licensed under CC BY-NC-SA 2.0.

Post last updated on 2022/06/05.