Categories
c casting malloc

Do I cast the result of malloc?

2689

In this question, someone suggested in a comment that I should not cast the result of malloc. i.e., I should do this:

int *sieve = malloc(sizeof(*sieve) * length);

rather than:

int *sieve = (int *) malloc(sizeof(*sieve) * length);

Why would this be the case?

3

  • 33

    stackoverflow.com/q/7545365/168175

    – Flexo

    Jan 15, 2012 at 20:06

  • 4

    Casts are evil. I see so many cast in code just as a result of bad coding practice. Whenever you need to insert one the first thing you should ask yourselves is ” what is wrong here” . Is everything declared as it should be ? If it is no cast would be needed so something is declared wrong. If you really do need to do some low level stuff on individual bytes in an int or so consider a union to access them. That’ll declare them just fine. As a rule of thumb do not insert them unless the compiler complains. Then avoid them. This example will not complain. void pointer will promote to any type.

    Feb 21, 2021 at 21:52


  • 2

    @HansLepoeter in C++ , those are necessary for malloc, giving some basis to my notion that there’s something wrong with it

    – An Ant

    Feb 27, 2021 at 11:51

2444

TL;DR

int *sieve = (int *) malloc(sizeof(int) * length);

has two problems. The cast and that you’re using the type instead of variable as argument for sizeof. Instead, do like this:

int *sieve = malloc(sizeof *sieve * length);

Long version

No; you don’t cast the result, since:

  • It is unnecessary, as void * is automatically and safely promoted to any other pointer type in this case.
  • It adds clutter to the code, casts are not very easy to read (especially if the pointer type is long).
  • It makes you repeat yourself, which is generally bad.
  • It can hide an error if you forgot to include <stdlib.h>. This can cause crashes (or, worse, not cause a crash until way later in some totally different part of the code). Consider what happens if pointers and integers are differently sized; then you’re hiding a warning by casting and might lose bits of your returned address. Note: as of C99 implicit functions are gone from C, and this point is no longer relevant since there’s no automatic assumption that undeclared functions return int.

As a clarification, note that I said “you don’t cast”, not “you don’t need to cast”. In my opinion, it’s a failure to include the cast, even if you got it right. There are simply no benefits to doing it, but a bunch of potential risks, and including the cast indicates that you don’t know about the risks.

Also note, as commentators point out, that the above talks about straight C, not C++. I very firmly believe in C and C++ as separate languages.

To add further, your code needlessly repeats the type information (int) which can cause errors. It’s better to de-reference the pointer being used to store the return value, to “lock” the two together:

int *sieve = malloc(length * sizeof *sieve);

This also moves the length to the front for increased visibility, and drops the redundant parentheses with sizeof; they are only needed when the argument is a type name. Many people seem to not know (or ignore) this, which makes their code more verbose. Remember: sizeof is not a function! 🙂


While moving length to the front may increase visibility in some rare cases, one should also pay attention that in the general case, it should be better to write the expression as:

int *sieve = malloc(sizeof *sieve * length);

Since keeping the sizeof first, in this case, ensures multiplication is done with at least size_t math.

Compare: malloc(sizeof *sieve * length * width) vs. malloc(length * width * sizeof *sieve) the second may overflow the length * width when width and length are smaller types than size_t.

52

  • 57

    Please consider updating the answer. The cast is no longer dangerous, and repeating oneself is not necessarily a bad thing (redundancy can help catch errors).

    Dec 8, 2016 at 12:30

  • 29

    Compilers have changed. An up-to-date compiler will warn you about a missing declaration of malloc.

    Dec 8, 2016 at 12:40

  • 93

    @n.m. Ok. I think it’s bad to assume that anyone reading here has a particular compiler. Also, since C11 the entire “implicit function” concept is gone, I didn’t know that. Still, I don’t see the point in adding a pointless cast. Do you also do int x = (int) 12; just to make things clear?

    – unwind

    Dec 8, 2016 at 12:48

  • 44

    @n.m. if explicitly casting a void pointer “helped” solve a bug, you more likely encountered undefined behavior, which would mean the program in question likely has a far worse, undiscovered bug that you haven’t run into yet. And one day, on a cold winter evening, you’ll come home from work to find your GitHub page flooded with issue reports complaining about demons flying out of the users’ noses

    Dec 17, 2016 at 18:49

  • 27

    @unwind Even I agree with you, (int)12 is not comparable. 12 is an int, the cast does simply nothing. The retval of malloc() is void *, not the pointer type casted to. (If it is not void *. So the analogy to (int)12 would be (void*)malloc(…) what nobody is discussing.)

    Jan 8, 2017 at 4:30

450

+50

In C, you don’t need to cast the return value of malloc. The pointer to void returned by malloc is automagically converted to the correct type. However, if you want your code to compile with a C++ compiler, a cast is needed. A preferred alternative among the community is to use the following:

int *sieve = malloc(sizeof *sieve * length);

which additionally frees you from having to worry about changing the right-hand side of the expression if ever you change the type of sieve.

Casts are bad, as people have pointed out. Especially pointer casts.

12

  • 113

    @MAKZ I’d argue that malloc(length * sizeof *sieve) makes it look like sizeof is a variable – so I think malloc(length * sizeof(*sieve)) is more readable.

    Apr 30, 2015 at 7:02

  • 33

    And malloc(length * (sizeof *sieve)) more readable still. IMHO.

    Aug 20, 2015 at 13:01

  • 23

    @Michael Anderson () issue aside, note that your suggested style switched the order., Consider when element count is computed like length*width, keeping the sizeof first in this case insures multiplication is done with at least size_t math. Compare malloc(sizeof( *ptr) * length * width) vs. malloc(length * width * sizeof (*ptr)) – the 2nd may overflow the length*width when width,length are smaller types that size_t.

    Dec 10, 2015 at 16:40


  • 3

    @chux it’s not obvious, but the answer has been edited so that my comment is less pertinent – the original suggestion was malloc(sizeof *sieve * length)

    Dec 11, 2015 at 0:18

  • 16

    C is not C++. Pretending that they are will ultimately lead to confusion and sadness. If you’re using C++, then a C-style cast is also bad (unless you’re using a very old C++ compiler). And static_cast>() (or reinterpret_cast<>() )is not compatible with any dialect of C.

    – David C.

    Feb 12, 2016 at 23:12


395

You do cast, because:

  • It makes your code more portable between C and C++, and as SO experience shows, a great many programmers claim they are writing in C when they are really writing in C++ (or C plus local compiler extensions).
  • Failing to do so can hide an error: note all the SO examples of confusing when to write type * versus type **.
  • The idea that it keeps you from noticing you failed to #include an appropriate header file misses the forest for the trees. It’s the same as saying “don’t worry about the fact you failed to ask the compiler to complain about not seeing prototypes — that pesky stdlib.h is the REAL important thing to remember!”
  • It forces an extra cognitive cross-check. It puts the (alleged) desired type right next to the arithmetic you’re doing for the raw size of that variable. I bet you could do an SO study that shows that malloc() bugs are caught much faster when there’s a cast. As with assertions, annotations that reveal intent decrease bugs.
  • Repeating yourself in a way that the machine can check is often a great idea. In fact, that’s what an assertion is, and this use of cast is an assertion. Assertions are still the most general technique we have for getting code correct, since Turing came up with the idea so many years ago.

23

  • 41

    @ulidtko In case you did not know, it’s possible to write code which compiles both as C and as C++. In fact most header files are like this, and they often contain code (macros and inline functions). Having a .c/.cpp file to compile as both is not useful very often, but one case is adding C++ throw support when compiled with C++ compiler (but return -1; when compiled with C compiler, or whatever).

    – hyde

    Mar 26, 2013 at 11:09


  • 40

    If someone had malloc calls inline in a header I wouldn’t be impressed, #ifdef __cplusplus and extern “C” {} are for this job, not adding in extra casts.

    – paulm

    May 6, 2013 at 17:55

  • 20

    Well, point 1 is irrelevant, since C != C++, the other points are also trivial, if you use the variable in your malloc call: char **foo = malloc(3*sizeof(*foo)); if quite full-proof: 3 pointers to char pointers. then loop, and do foo[i] = calloc(101, sizeof(*(foo[i])));. Allocate array of 101 chars, neatly initialized to zeroes. No cast needed. change the declaration to unsigned char or any other type, for that matter, and you’re still good

    Dec 23, 2013 at 15:37

  • 39

    When I tought I got it, there it comes! Fantastic answer. Its the first time here in StackOverflow that I +1 two opposite answers! +1 No, you dont cast, and +1 Yes, you do cast! LOL. You guys are terrific. And for me and my students, I made my mind: I do cast. The kind of errors students make are more easily spotted when casting.

    – DrBeco

    Sep 26, 2014 at 3:22

  • 18

    @Leushenko: Repeating yourself in a way that cannot be validated by machine nor by local inspection is bad. Repeating yourself in ways that can be validated by such means is less bad. Given struct Zebra *p; ... p=malloc(sizeof struct Zebra);, the malloc can’t avoid duplciating information about p’s type, but neither the compiler nor local code inspection would detect any problem if one type changed but the other didn’t. Change the code to p=(struct Zebra*)malloc(sizeof struct Zebra); and the compiler will squawk if the cast type doesn’t match p, and local inspection will reveal…

    – supercat

    Sep 18, 2015 at 17:46