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
.
Just about every modern operating system will recover all the allocated memory space after a program exits. The only exception I can think of might be something like Palm OS where the program's static storage and runtime memory are pretty much the same thing, so not freeing might cause the program to take up more storage. (I'm only speculating here.)
So generally, there's no harm in it, except the runtime cost of having more storage than you need. Certainly in the example you give, you want to keep the memory for a variable that might be used until it's cleared.
However, it's considered good style to free memory as soon as you don't need it any more, and to free anything you still have around on program exit. It's more of an exercise in knowing what memory you're using, and thinking about whether you still need it. If you don't keep track, you might have memory leaks.
On the other hand, the similar admonition to close your files on exit has a much more concrete result - if you don't, the data you wrote to them might not get flushed, or if they're a temp file, they might not get deleted when you're done. Also, database handles should have their transactions committed and then closed when you're done with them. Similarly, if you're using an object oriented language like C++ or Objective C, not freeing an object when you're done with it will mean the destructor will never get called, and any resources the class is responsible might not get cleaned up.
Best Answer
When an object is "double-freed", the most common cause is that you're (unnecessarily) releasing an autoreleased object, and it is later autoreleased when the containing autorelease pool is emptied.
I've found that the best way to track down the extra release is to use the NSZombieEnabled environment variable for the affected executable in Xcode. For a quick rundown of how to use it, check out this CocoaDev wiki page. (In addition to this page, Apple has documented some incredibly obscure yet useful tips for debugging code in Xcode, some of which have saved my bacon more than a few times. I suggest checking out this Technical Note on developer.apple.com — link jumps to the section on Cocoa's Foundation framework).
Edit: You can often track the offending object down within the Xcode debugger, but it's often much easier if you use Instruments to assist you. From Xcode, choose Run → Start With Performance Tool → Object Allocations and you should be able to trace the offending object back to where it was created. (This will work best if you're enabled zombies as discussed above.) Note: Snow Leopard adds a Zombies tool to Instruments, accessible from the Run menu as well. Might be worth the $29 alone! ;-)
There is also a related SO question here.