TL;DR

You should use

int *p = malloc(n * sizeof *p);

for two reasons

1. The cast (int*) is not necessary, which means it's clutter.
2. Using sizeof *p instead of sizeof(int) removes code duplication.

But remember to check if allocation succeeded before using the memory. It's done like this:

int *p = malloc(n * sizeof *p);
if(!p) {
    // Handle error
}

Longer answer

1 - Casting

Some people argue that the cast makes it possible to compile the code with both a C compiler and a C++ compiler. While this is technically true and sometimes useful, it's not the typical use case. If you know that you want to be able to do this, then cast. A C++ compiler will throw a compiler error if you don't.

But in C, a void pointer (malloc returns a void pointer^[1]) can safely be implicitly casted to any other pointer type and back. So it is completely safe to omit it. Note that this is true for void pointers in general. It's not special for malloc. It just happens to be the case that this discussion comes up a lot when talking about malloc.

2 - sizeof

Let's say you have this code:

int *p;
// Many lines of code
p = malloc(size1 * sizeof(int));
// More lines of code
p = malloc(size2 * sizeof(int)); 

Suddenly you realize that you have to change the type of p to another pointer type. Will you remember to change everywhere? And are you sure you will not miss anything? Using sizeof *p eliminates this problem. But do remember that sizeof p is the size of the pointer, that is sizeof (int*). Mixing this up might give you annoying and hard traced bugs.

If you have a pointer to pointer to create a 2D structure, the pattern is like this:

int **p;
p = malloc(x * sizeof *p);
for(int i=0; i<x; i++) 
    p[i] = malloc(y * sizeof *p[i]);

If you're dealing with pointers to arrays, you might want to be a bit careful. Especially if they are arguments to functions. For instance, int a[5][5] means different things if it is declared as an argument or in function body or global space. If declared in function body or global space, that will give you a two dimensional 5x5 array. But if declared in a function argument, it will be a pointer to one dimensional 5 array. The equivalent declaration is int (*a)[5].

There is also the case with flexible array members of a struct, but IMO it's pretty obvious that this method does not work flawlessly there. It also does not work for void pointers, because they cannot be dereferenced.

Read more about when this work and not here

More opinionated stuff

Other argue that "it's good habit" to add that extra check that the cast gives. It forces you to think one more time. I strongly disagree with this for several reasons.

Firstly, you very rarely do this for non-pointer types. This code looks completely ridiculous:

signed char x = (signed char)42;
long y = (long)x - (long)8;

Do note that these casts do have their uses. What I'm saying is that it is a bad thing to blindly throw them in everywhere without even knowing why or if it's needed.

Secondly, in C a cast typically means "I know what I'm doing". So if you're doing it wrong, you can actually HIDE a bug. The argument about forcing you to think again makes sense in C++, because it will not compile if you do it wrong.

posted almost 3 years ago

CC BY-SA 4.0

3y ago

klutt‭

1273 reputation 17 12 162 52