C programming malloc and NULL

Question

I am very new to C programming and am working on a project for school that involves a binary search tree. In the instructions, it states that we must call malloc for each node in the tree.

When I declare the head node, is there a difference between the following statements? If so, which is the proper way? I feel like the first is the correct way:

    struct Node* head = NULL; 
    head = (struct Node*) malloc(1 * sizeof(struct Node));

and

    struct Node* head = (struct Node*) malloc(1 * sizeof(struct Node));
    head = NULL;

[Do I cast the result of malloc? **No.**](https://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc) — Deduplicator, Sep 29 '18 at 17:10
@drumGod31: you can accept one of the answers by clicking on the grey checkmark below its score. — chqrlie, Oct 11 '18 at 20:46

wizzwizz4 · Answer 1 · 2018-09-29T16:53:41.453

Version B is a more concise version of Version A, if you remove the second line (with = NULL) because that undoes all the work you did on the first line.

Let's break it down, and start with (struct Node*) malloc(1 * sizeof(struct Node)). The malloc call allocates a certain amount of memory, which is 1 times the size of a struct Node. The beginning part of that just says "treat this as a pointer to a struct Node"; without this, it's just treated as a "pointer to... something" (aka a void *).

You don't actually need that (struct Node *) at the beginning, since the variable you're assigning it to is a struct Node * variable, but some people prefer it. I personally don't write it, because it stops the computer warning me about a certain type of error caused by a typo that I make frequently.

Now to the differences between the versions. Version A:

Sets the pointer to NULL.
Allocates some memory.
Sets the pointer to point to that memory.

Version B:

Allocates some memory.
Sets the pointer to point to that memory.
Sets the pointer to NULL...

You can see why this would be a problem.

You have a pointer to NULL instead of to somewhere you can store data; and
You have claimed some memory for your program but have no way of freeing it; your program has leaked memory.

This is bad. I recommend using version B, but only the first line from it; never ever ever set a malloc'd pointer to NULL without freeing it first or your program will slowly break.

chux - Reinstate Monica · Answer 2 · 2018-09-29T17:48:14.670

"If so, which is the proper way?" implies at least 1 of the 2 is of them is a good choice.

Use these axioms

Prefer dry vs. wet code. Drop the cast. No need to re-assign a variable that has not been used.
Resource acquisition is initialization. Initialize a variable when possible.
Check allocation success. Did the *alloc() call succeed?
Allocate to the size of the referenced object, not the type. Easier to code right, review and maintain.

Let us consider a 3rd alternative as the 2 OP code snippets do not well follow all of those axioms.

struct Node* head = malloc(sizeof *head); 

// Check for allocation failure
if (head == NULL) {
  // Handle out of memory in some fashion
  fprintf(stderr, "Out of memory\n");
  exit(EXIT_FAILURE);
}

@chqrlie: Or `return EXIT_FAILURE;` if this is the `main` function. — ShadowRanger, Sep 29 '18 at 17:44

Bart Friederichs · Answer 3 · 2018-09-29T16:44:20.433

1

The first one is correct. The second one leads to memory leaks and (possibly; if you continue to use head) to segfaults.

In case of the second one, you assign a pointer to a piece of code returned from malloc to head. And right afterwards, you overwrite that value with NULL, causing the head variable to be basically useless. At the same time, you lose the pointer to the mallocd memory. You can never use or reclaim that memory, which is called a "memory leak".

edited Sep 29 '18 at 16:44

answered Sep 29 '18 at 16:42

Bart Friederichs

33,050
15
95
195

why does the second lead to memory leaks? – drumGod31 Sep 29 '18 at 16:43
1

You overwrite the value, losing it. And then you can never `free()` that memory. – Bart Friederichs Sep 29 '18 at 16:45

rcgldr · Answer 4 · 2018-09-29T22:13:03.437

1

The first one is "correct", but does an unnecessary assignment of head = NULL, since it's immediately followed by a second assignment of head = ... malloc(...), overwriting the NULL assignment. Note that malloc() will return NULL if it fails to allocate memory.

edited Sep 29 '18 at 22:13

answered Sep 29 '18 at 16:50

rcgldr

27,407
3
36
61

chqrlie · Answer 5 · 2018-09-29T20:14:10.793

In the second version, you overwrite the pointer value obtained from malloc() with NULL and therefore cannot access this memory anymore, a situation called a memory leak: definitely not what you want.

The first version is not incorrect, but initializing the pointer to NULL is redundant since you immediately store the return value of malloc() into it. The compiler will probably omit this redundant store, but for readability, you can simplify it as:

struct Node *head = (struct Node*)malloc(1 * sizeof(struct Node));

Note that casting the return value of malloc is useless in C and somewhat error prone. It is recommended to just write:

struct Node *head = malloc(1 * sizeof(struct Node));

As a matter of fact, the 1 * is usually omitted:

struct Node *head = malloc(sizeof(struct Node));

Note that it is also error prone to use an explicit type in the malloc argument as the C compiler will not perform any kind of consistency check between the size allocated and the pointer type. If you later change the type of head or use a different type by mistake, the size may be incorrect. For a safer way to specify the size to allocate, use the target type of the destination pointer:

struct Node *head = malloc(sizeof(*head));

For an even safer alternative, use calloc() that will initialize the memory to all bits 0, which on most current hardware is the zero value for integer and floating point members and the null pointer value for all pointer types. One more advantage is you can specify the number of items to allocate, as you seem to like:

struct Node *head = calloc(1, sizeof(*head));  // all members initialized to zero

In all cases, testing if the memory allocation succeeded is highly recommended:

struct Node *head = calloc(1, sizeof(*head));  // all members initialized to zero
if (head == NULL) {
    fprintf(stderr, "memory allocation failed\n");
    exit(EXIT_FAILURE);
}

You could further simplify the expression with sizeof *head instead of sizeof(*head). These are equivalent, the parentheses are redundant. I personally omit them only for a naked identifier, as in sizeof head, but use them for any other expression. Conversely, parentheses are required for a type, as in sizeof(struct Node)

score 0 · Answer 6 · edited Sep 29 '18 at 16:51

All statements in 'c' are executed sequntially (except from jumps, like for or while loops).

So, int the first fragment you declared the variable 'head' and assigned it a value of NULL, secondly, you allocated memory and assigned its address to the head, overriding the previous value of NULL. Now you can use the 'head' variable to point to the allocated memory in the program.

In the second fragment you did an opposit. You allocated memory first and assigned its address to head, then you overwrote this value with NULL. Now you cannot point to the allocated memory since you have lost the pointer. Your program will not work and your memory leaked, since it cannot be freed later (becais it has no pointer info to it).

score 0 · Answer 7 · answered Sep 29 '18 at 16:54

You can even just write struct Node* head = (struct Node*) malloc(sizeof(struct Node)) without the NULL initialisation since you declare and then directly allocate memory. Use bzero(head, sizeof(struct Node)); (include strings.h) if you actually want to zero-fill what you previously allocated.

The second method is just wrong because now head points to NULL and there's no more reference to the previously allocated memory, resulting in memory leaks.

C programming malloc and NULL

7 Answers7