C - Calloc instead of Malloc to save lines?

Question

I just wanted to double check that the following was a totally okay and appreciated improvement:

If I were to have:

struct ll_Node
{
    void *data;
    struct ll_Node *next, *prev;
};

And I had the constructor:

static struct ll_Node* ll_Node_new(void *data)
{
    struct ll_Node *node = calloc(1, sizeof(struct ll_Node));
    if (node == NULL)
        exit(EXIT_FAILURE);
    node->data = data;
    return node;
}

In the above code snippet, I want to make sure the previous variable is set to NULL. I heard using calloc defaults variables to 0 in memory which in C for all intents and purposes is the exact same thing as NULL.

So instead of:

struct ll_Node *node = malloc(sizeof(struct ll_Node));

I do

struct ll_Node *node = calloc(1, sizeof(struct ll_Node));

I ask this because calloc seems to be extremely popular when it comes to arrays, because if you were to use malloc for an array, the every item would have garbage data in it. Is using calloc with a 1 in the first parameter slot considered bad coding?.

Tiny tiny subquestion: is exiting the program if the thing I use calloc on is NULL acceptable? Or is that a bit extreme? I figure that if there's no memory left to create a struct then clearly any hopes of my program continuing to run are forfeit.

Pass 1 is ok, no surely bad coding. About exiting the program on `calloc` returning `NULL`: you should take care of already allocated items: you must `free` them before to exit your program or it will leak memory. — LPs, Aug 24 '16 at 14:46
A null pointer may not have all bits set to zero, so if you're writing portable code, calloc is a trap. — 2501, Aug 24 '16 at 14:47
@LPs since the program is already on a fast train to terminate, why bother with manually freeing memory? The OS will do that anyway. — bolov, Aug 24 '16 at 14:49
OT: Coding `struct ll_Node *node = calloc(1, sizeof *node);` is more robust. — alk, Aug 24 '16 at 14:49
@LPs How would I, after every single `malloc` and `calloc` in my entire program somehow make a function that could go through every variable and free them like that? I don't see how that can be done without being a mess. — Hatefiend, Aug 24 '16 at 14:50
@Hatefiend: "*How would I [...] make a function that could go through every variable and free them like that? I don't see how that can be done without being a mess.*" Sry to state this, but you then might already have a mess now ... — alk, Aug 24 '16 at 14:52
@bolov You are assuming an, let me say, _hi-level_ OS. I didn't ;) — LPs, Aug 24 '16 at 14:53
@alk - I'm not sure that's completely fair. Error-handling (or more specifically, rigorous resource cleanup in the face of errors) is just an unpleasant problem in C, given the lack of RAII, etc.. — Oliver Charlesworth, Aug 24 '16 at 14:53
Exit on 'no space' is fine in code that you're writing for your program to use. It is not OK for code that you expect other people to use. You'd be a bit upset if your program exited because a library function ran out of space. That said, there often aren't many alternatives to exiting. But it should be at the decision of the programmer. When it's you deciding for your code, exit is fine — I do that sort of thing. When you're deciding for others, it is presumptuous — you might have just thrown away the user's hour of editing when the file could have been saved. It would make you unpopular. — Jonathan Leffler, Aug 24 '16 at 14:54
@alk Well it's just that it's already a pain to check every EVERY `malloc` and `calloc` if the pointer returned is `NULL`, let alone to have to locate and de-allocate every pointer nicely. This is going to sound really stupid but would making a `LinkedList` of `void*` and have it contain every single variable I've ever allocated memory for in the runtime be a wise way of solving that problem? — Hatefiend, Aug 24 '16 at 14:54
@JonathanLeffler But if the program ran out of memory, how could it possibly recover? It can no longer create new memory for variables so even something as simple as saving their data to a `FILE` might be impossible. — Hatefiend, Aug 24 '16 at 14:56
You can write wrapper functions: `extern void *emalloc(size_t size) { void *space = malloc(size); if (space == 0) error("Out of memory allocating %zu bytes\n", size); return space; }` where I assume `error()` does not return and does exit. — Jonathan Leffler, Aug 24 '16 at 14:56
@Hatefiend it didn't ran out of memory. It ran out of enough memory to allocate the requested size. It is possible it could still alocate smaller sizes, or even more total memory, but not contiguous. — bolov, Aug 24 '16 at 14:58
It may have pre-allocated the space it needs to ensure that it can save the results. The writing process may not need any more memory. Word processors, for example, take steps to ensure that they can at least not lose your precious work even if they run out of memory. A lot of my code uses a set of functions that do exit on error — it isn't a wholly unreasonable strategy. But if the function is going to be a general-purpose library function for other programmers to use, I avoid exiting on error (it ceases to be fully general-purpose when it makes decisions about how to respond to errors). — Jonathan Leffler, Aug 24 '16 at 15:00
even with no errors with `malloc` or `calloc`, how were you planning on deleting/inserting/finding nodes, clearing the entire list,, etc? Every linked-linked implementation should have these function calls; you already have to keep track of EVERY node you create, or what are you even doing?. Then if `malloc` errs out and you want to clean up and exit, it should be as simple as `delete_list(); exit(errCode);` — yano, Aug 24 '16 at 15:02
Always be prepared for the perfect "dismount": https://www.youtube.com/watch?v=CzycofSj7EM ;-) — alk, Aug 24 '16 at 15:03
Oh, and exiting without an error message explaining why you're exiting is bad. Your user will only be able to say "the program stopped". But that in turn indicates part of the problem with the code exiting. I report errors with `fprintf(stderr, "Out of memory\n");` but in a GUI system, that's not appropriate. Maybe the error message should be written to a log file, maybe `syslog`, or maybe an alert box should be should be shown, or … Your library function can't do all of those; it can't tell which of those it should do. It's another reason not to exit from within a library function. — Jonathan Leffler, Aug 24 '16 at 15:03
More or less safe (in terms of graceful error handling) C code is *at least* 30% error checking & handling. — alk, Aug 24 '16 at 15:06

score 2 · Accepted Answer · answered Aug 25 '16 at 02:39

The answers to your questions may all come down to style.

Although your use of calloc is correct to ensure that the allocated memory is zeroed out, especially in a case like this where you are not assigning all the fields when constructing the node, I dislike that you are relying on calloc setting the 0s instead of specifically assigning prev and next to NULL. You have to consider someone else (or even you) reading this code 6 months from now. The code's intention is slightly less clear by not explicitly assigning prev and next to NULL.

Something like this:

static struct ll_Node* ll_Node_new(void *data)
{
    struct ll_Node *node = calloc(1, sizeof(struct ll_Node));
    if (node == NULL) {
        exit(EXIT_FAILURE);
    }
    node->data = data;
    node->prev = node->next = NULL;
    return node;
}

Makes it clear to a reader that prev and next are initialized to NULL and the caller should make sure they are set to the correct values. Code is going to be read many more times than written, so anything you can do to make the intentions of the code obvious will put less stress on someone reading it.

As to your other question, I agree with the commenters that it is preferable if your function returns and you verify in a single place if the function was successful or not and do any notification (setting an error string/code) and cleanup (freeing up the resources cleanly) necessary in a single location.

Thank you for your advice. I actually didn't know you can do multiple assignments on the same line. That's really cool! — Hatefiend, Aug 26 '16 at 12:50

score 1 · Answer 2 · edited May 23 '17 at 12:08

Using calloc(1, sz) is quite acceptable, and in fact preferable to malloc(sz) followed by memset. See here for further reading.

However, it is a bit sloppy to assume that calloc generates null pointers. While that is true on the most popular modern platforms, there have been platforms in the past (and perhaps even still existing) where the null pointer has some other bit pattern.

To be as correct as possible it'd be better to calloc and then explicitly set any pointer to NULL. If you are on a platform where null pointers are in fact all-bits-zero then the compiler will optimize out those assignments.

C - Calloc instead of Malloc to save lines?

2 Answers2