When to check the return value of malloc

[CBFalconer]

Eric Sosman wrote

This thread was useful, now I know I never have to buy extra
memory again.

PROVIDED you malloc it in 20 byte chunks. Since the standard
specifies that freed memory be made available again, you must be
perfectly safe in allocating 4k by:

for (i = 0; i < 20; i++) a = malloc(20);
for (i = 0; i < 20; i++) free(a);
ptr = malloc(4000);

with suitable declarations for a, i, ptr, and all the needed
#includes. Learning is wunnerful.

 

[user923005]

Howdy,

I was reflecting recently on malloc.

Obviously, for tiny allocations like 20 bytes to strcpy a filename or
something, there's no point putting in a check on the return value of
malloc.

OTOH, if you're allocating a gigabyte for a large array, this might
fail, so you should definitely check for a NULL return.

So somewhere in between these extremes, there must be a point where you
stop ignoring malloc's return value, and start checking it.

Where do people draw this line? I guess it depends on the likely system
the program will be deployed on, but are there any good rule-of-thumbs?

The good rule of thumb is always check.

True story about an OS/2 application for fielding telephone messages:
While I was working as a subcontracor at a large corporation nearby
some 18 years ago or so, there was an OS/2 application that did
zillions of 16 byte allocations and frees. No big "gimmie a
megabyte!" demands at all. The machine had 4 MB {IIRC}, which at the
time was *cough* a lot of memory. Now, according to the statistics I
examined, the packet creation and packet deletion was at a sort of
homeostatis that should have never seen a failure. However, the
memory allocater for OS/2 had a bad habit of fragmenting memory in
some very bizarre ways if there were lots and lots of very rapid
malloc()/free() pairs. The end result is that those 16 byte
allocation requests would eventually start to fail. Since there was
no check for malloc() success in the code [I DIDN'T WRITE IT!] it
crashed heinously. My first stab at fixing it (after putting in
diagnosics to at lest describe the problem before exiting) was to call
a non-standard "CompactMemory()" routine at regular intervals. At
first, it seemed to fix the problem but it lead to to other problems.
The system had to be near real time, and the compact memory call would
freeze it up momemtarily and also, it would eventually start to fail
again anyway (even though it would run much longer before failure).
My final solution was to allocate most of available RAM into a single
block and then I wrote my own sub-allocator. It was actually very
simple because all the packets were the same size, and so I just
tagged them as free or in use with an array of bits.

So, even if you do not allocate any big memory hunks and even if you
know that all of the allocations should succeed, you should still
check them. Because things don't always behave the way that you know
that they should.

IMO-YMMV.

 

[user923005]

Eric Sosman wrote

This thread was useful, now I know I never have to buy extra
memory again.

PROVIDED you malloc it in 20 byte chunks.  Since the standard
specifies that freed memory be made available again, you must be
perfectly safe in allocating 4k by:

   for (i = 0; i < 20; i++) a = malloc(20);
   for (i = 0; i < 20; i++) free(a);
   ptr = malloc(4000);

with suitable declarations for a, i, ptr, and all the needed
#includes.  Learning is wunnerful.

You might be perfectly safe to allocate (say) 64K according to the ISO
C Standard. But the other program that is running and has consumed
all but 19 free bytes before your program executes the first malloc()
doesn't know that.

 

[user923005]

PROVIDED you malloc it in 20 byte chunks.  Since the standard
specifies that freed memory be made available again, you must be
perfectly safe in allocating 4k by:

   for (i = 0; i < 20; i++) a = malloc(20);
   for (i = 0; i < 20; i++) free(a);
   ptr = malloc(4000);

with suitable declarations for a, i, ptr, and all the needed
#includes.  Learning is wunnerful.

You might be perfectly safe to allocate (say) 64K according to the ISO
C Standard.  But the other program that is running and has consumed
all but 19 free bytes before your program executes the first malloc()
doesn't know that.- Hide quoted text -

- Show quoted text -

Sorry, sarcasm detector switch was broken off, and laying on the
floor.

 

[Ulrich Eckhardt]

I was reflecting recently on malloc.

Obviously, for tiny allocations like 20 bytes to strcpy a filename or
something, there's no point putting in a check on the return value of
malloc.

OTOH, if you're allocating a gigabyte for a large array, this might
fail, so you should definitely check for a NULL return.

So somewhere in between these extremes, there must be a point where you
stop ignoring malloc's return value, and start checking it.

Where do people draw this line? I guess it depends on the likely system
the program will be deployed on, but are there any good rule-of-thumbs?

As you have seen, the general attitude is to always check, and I actually am
one of that crowd. However, I do it slightly different:

void* xmalloc(size_t s) {
void* res = malloc(s);
if(!res) {
fprintf( stderr, "malloc() failed\n");
exit(EXIT_FAILURE);
}
return res;
}

Just like that, it stands in every program that is simply not prepared to
handle OOM conditions. In the code then, when I e.g. allocate temporary
storage to compose a filename, I just call xmalloc() and never check its
result, because I know that it either succeeded or exit()ed. Now, for
larger allocations I might still use malloc() and check the result, but I
mostly don't bother.

Note that this strategy is not always applicable, typically only to
short-running programs (like e.g. a compiler) but not to e.g. server
programs where the failure must be limited to the particular request only.
(But I wouldn't write those in C either...).

Further, the same strategy can of course be applied to other functions like
realloc(), fopen() etc. Whenever you can't sensibly recover from a resource
shortage, just exit with a meaningful error message.

Uli

 

[Randy Howard]

Eric Sosman wrote
Marty James wrote:

I was reflecting recently on malloc.

Obviously, for tiny allocations like 20 bytes to strcpy a
filename or something, there's no point putting in a check on
the return value of malloc.

     "Obviously," you can allocate an infinite amount of
memory as long as you get it in 20-byte chunks?  Did you
used to work for Enron or something?

This thread was useful, now I know I never have to buy extra
memory again.

PROVIDED you malloc it in 20 byte chunks.  Since the standard
specifies that freed memory be made available again, you must be
perfectly safe in allocating 4k by:

   for (i = 0; i < 20; i++) a = malloc(20);
   for (i = 0; i < 20; i++) free(a);
   ptr = malloc(4000);

with suitable declarations for a, i, ptr, and all the needed
#includes.  Learning is wunnerful.

You might be perfectly safe to allocate (say) 64K according to the ISO
C Standard. But the other program that is running and has consumed
all but 19 free bytes before your program executes the first malloc()
doesn't know that.

Both of you need to get a sense of humor.

 

[Bart C]

Howdy,

I was reflecting recently on malloc.

Obviously, for tiny allocations like 20 bytes to strcpy a filename or
something, there's no point putting in a check on the return value of
malloc.

OTOH, if you're allocating a gigabyte for a large array, this might
fail, so you should definitely check for a NULL return.

So somewhere in between these extremes, there must be a point where you
stop ignoring malloc's return value, and start checking it.

Where do people draw this line? I guess it depends on the likely system
the program will be deployed on, but are there any good rule-of-thumbs?

If checking is that much trouble then create a wrapper function around
malloc() that will always return a valid result. (The wrapper will check for
NULL results of malloc() and abort or do other exception code.) Then call
that instead of malloc(). This way you dispense with checking every time.

Use this when allocation failure is (a) unimportant or (b) very important
(that the program cannot proceed). (Seems paradoxical I know.)

To take some other action when allocation fails then call malloc() in the
regular way.

Bart

 

[Malcolm McLean]

Obviously, for tiny allocations like 20 bytes to strcpy a filename or
something, there's no point putting in a check on the return value of
malloc.

OTOH, if you're allocating a gigabyte for a large array, this might
fail, so you should definitely check for a NULL return.

So somewhere in between these extremes, there must be a point where you
stop ignoring malloc's return value, and start checking it.

Where do people draw this line? I guess it depends on the likely system
the program will be deployed on, but are there any good rule-of-thumbs?

Yes.
Imagine you've got 2GB installed and are allocating 20 bytes. The system is
stressed and programs crash or terminate for lack of memory once a day. Any
more than that, and no-one would tolerate it.
So the chance the crash being caused by your allocation is 1/ 100 000 000,
or once every several hundred thousand years. The chance of the computer
breaking during this period is so so much higher, there is in this case no
point checking the malloc().

It is elementary. When you control the quality of a part, and there are
costs - here increased complexity of code, thus maintenance costs - the
quality should be high enough that your part is unlikely to the the point of
failure, but no higher.

As others have pointed out, if your main allocation is in a loop, the
probabilities have to be adjusted accordingly.

You might be interestwed in xmalloc(), on my website, which gets round this
problem of too much error-handling code which will never be executed. For
all I have said, there is also a case for making programs which are correct.

 

[Army1987]

There's an old saying: Never check for an error condition you don't
know how to handle.

But if you can't figure out what to do, you can always just terminate
the program. It's not necessarily the best thing you can do, but it's
the second simplest, and it's almost certainly better than the
simplest (ignoring the error).

Well, I don't usually check the result of a call such as
fprintf(stderr, "Can't frobnicate %s: %s\n", frob, strerror(errno)),
because I don't know what should I do if it failed, but I don't think that
just terminating the program would be a good idea (unless I were going to
terminate it right after the fprintf regardless of its success, that is).

 

[Flash Gordon]

Malcolm McLean wrote, On 19/01/08 10:47:

Yes.
Imagine you've got 2GB installed and are allocating 20 bytes. The system
is stressed and programs crash or terminate for lack of memory once a
day. Any more than that, and no-one would tolerate it.
So the chance the crash being caused by your allocation is 1/ 100 000
000, or once every several hundred thousand years. The chance of the
computer breaking during this period is so so much higher, there is in
this case no point checking the malloc().

This is incredibly bad advice. It has also been pointed out to Malcolm
in the past that it is incredibly bad advice.

I run out of memory on my company notebook with 2GB of RAM. I know
people run out of memory on servers with far more than 2GB of RAM. In
fact, I don't think I've had a month when some piece of SW has not
reported being out of memory and provided a recovery mechanism.

You might be interestwed in xmalloc(), on my website, which gets round
this problem of too much error-handling code which will never be
executed. For all I have said, there is also a case for making programs
which are correct.

I would suggest looking very carefully at any malloc wrapper before
using it. You need to decide whether aborting on failure is appropriate
or some recovery strategy.

 

[Army1987]

Ulrich Eckhardt wrote:

[When should the result of malloc() be checked?]

As you have seen, the general attitude is to always check, and I actually am
one of that crowd. However, I do it slightly different:

void* xmalloc(size_t s) {
void* res = malloc(s);
if(!res) {
fprintf( stderr, "malloc() failed\n");

Someone once suggested to make it something like
fprintf( stderr, "malloc(%zu) failed\n", s);
(or %lu and (unsigned long)s for C90), so that you will immediately notice
something strange if e.g. you xmalloc(-1) by mistake.

 

[Ulrich Eckhardt]

You might be interestwed in xmalloc(), on my website[1], which gets round
this problem of too much error-handling code which will never be executed.

I took a look at it. Apart from being too complicated for most programs
(yes, some will actually be able to use the additional complexity), it has
one IMHO grave bug: it uses 'int' for the allocation size. Use size_t,
which is the correct type, or do you check if the result of 'strlen()' can
be safely converted to an 'int' before calling your 'xmalloc()'?

- sorry, no chocolate for you -

Uli

[1] http://www.personal.leeds.ac.uk/~bgy1mm/XMalloc/xmalloc.html

 

[CBFalconer]

Eric Sosman wrote
Marty James wrote:

I was reflecting recently on malloc.

Obviously, for tiny allocations like 20 bytes to strcpy a
filename or something, there's no point putting in a check on
the return value of malloc.

"Obviously," you can allocate an infinite amount of
memory as long as you get it in 20-byte chunks? Did you
used to work for Enron or something?

This thread was useful, now I know I never have to buy extra
memory again.

PROVIDED you malloc it in 20 byte chunks. Since the standard
specifies that freed memory be made available again, you must be
perfectly safe in allocating 4k by:

for (i = 0; i < 20; i++) a = malloc(20);
for (i = 0; i < 20; i++) free(a);
ptr = malloc(4000);

with suitable declarations for a, i, ptr, and all the needed
#includes. Learning is wunnerful.

You might be perfectly safe to allocate (say) 64K according to
the ISO C Standard. But the other program that is running and
has consumed all but 19 free bytes before your program executes
the first malloc() doesn't know that.

No, you haven't been following. The OP postulated that assignment
of 20 bytes was safe, and needed no checking. He wanted to know a
level that needed checking. I was pointing out that his original
assumption obviated the need for ANY checking of malloc.

 

[Malcolm McLean]

This is incredibly bad advice. It has also been pointed out to Malcolm in
the past that it is incredibly bad advice.

Advice is a dangerous gift, even from the wise to the wise.

I run out of memory on my company notebook with 2GB of RAM. I know people
run out of memory on servers with far more than 2GB of RAM. In fact, I
don't think I've had a month when some piece of SW has not reported being
out of memory and provided a recovery mechanism.

Yes, but not very often on a single allocation of 20 bytes. That will happen
once
in every 100 000 000 months on such a machine.

I would suggest looking very carefully at any malloc wrapper before using
it. You need to decide whether aborting on failure is appropriate or some
recovery strategy.

xmalloc() never returns null, so there is no need to provide error-checking.
Basically the idea is to get clutter which will never be executed out of
functions, so that normal logic flow stands out more clearly.
Obviously you cannot guarantee an infitie supply of memory. So by default
xmalloc() aborts with an error message. That's because there is not much
else you can do within the constraints of ANSI C.
However the failure handler can be reset, and I have one for X which
requests that the user terminate some other application. Again, that isn't
approriate for everything - a server, for instance, could release memory
from an emergency store, and then put itself into "critical please attend to
me mode".

 

[Malcolm McLean]

You might be interestwed in xmalloc(), on my website[1], which gets round
this problem of too much error-handling code which will never be
executed.

I took a look at it. Apart from being too complicated for most programs
(yes, some will actually be able to use the additional complexity), it has
one IMHO grave bug: it uses 'int' for the allocation size. Use size_t,
which is the correct type, or do you check if the result of 'strlen()' can
be safely converted to an 'int' before calling your 'xmalloc()'?

- sorry, no chocolate for you -

If the amount of memory won't fit into an int you probably shouldn't be
calling the function. It is for small amounts of memory, and is compatible
with the original C specification of malloc().

Also, using a signed value means that we can pick up many bugs. If garbage
is passed to malloc() then 50% of the time the argument will be negative.
Assuming randomness, if the function is called more than once or twice, you
practically have a guarantee of catching the bug.

I can't do anything about the size_t returned from strlen. It is most
unlikely that you'll have strings of more than the range of an int, and a
lot of people complain at use of high bit types - that's the objection, and
it is a legitimate one, made most often to the campaign for 64 bit ints.

 

[Flash Gordon]

Malcolm McLean wrote, On 19/01/08 13:44:

Advice is a dangerous gift, even from the wise to the wise.

Irrelevant. Your advice was bad for anyone whether they recipient is
wise or not.

Yes, but not very often on a single allocation of 20 bytes. That will
happen once
in every 100 000 000 months on such a machine.

If my machine is out of memory then an allocation request for ONE byte
will fail, that it is other programs using up all of the memory is
irrelevant. I run out of memory on my machine regularly as stated.
Fortunately even MS seems to have a better understanding of this than you.

xmalloc() never returns null, so there is no need to provide

<snip>

So are you claiming that the OP should not look carefully at the malloc
wrapper and deciding on the appropriate strategy for his/her
application? If not I don't see the point of that message.

 

[Flash Gordon]

Malcolm McLean wrote, On 19/01/08 13:51:

You might be interestwed in xmalloc(), on my website[1], which gets
round
this problem of too much error-handling code which will never be
executed.

I took a look at it. Apart from being too complicated for most programs
(yes, some will actually be able to use the additional complexity), it
has
one IMHO grave bug: it uses 'int' for the allocation size. Use size_t,

I could have guessed that. You will find the same problem throughout the
code Malcolm produces and recommends.

If the amount of memory won't fit into an int you probably shouldn't be
calling the function.

You mean definitely, since it will needlessly fail.

It is for small amounts of memory, and is

Then document it as not being a general purpose wrapper. Or better yet
fix it.

compatible with the original C specification of malloc().

You mean it returns a char* instead of a void*?

Also, using a signed value means that we can pick up many bugs.

I can pick them up without that. If I know a program should only be
allocating less than a specific amount I can put that check in the
malloc wrapper.

If
garbage is passed to malloc() then 50% of the time the argument will be
negative. Assuming randomness, if the function is called more than once
or twice, you practically have a guarantee of catching the bug.

I've seen plenty of such assumptions break in the real world. In fact,
an ex-colleague made a similar (not identical) set of assumptions when
changing our libraries and as a result customers found that our SW would
crash.

I can't do anything about the size_t returned from strlen. It is most
unlikely that you'll have strings of more than the range of an int, and
a lot of people complain at use of high bit types - that's the
objection, and it is a legitimate one, made most often to the campaign
for 64 bit ints.

It means again that your function is NOT a general purpose malloc
wrapper and not suitable for use by a lot of people.

 

[Eric Sosman]

As you have seen, the general attitude is to always check, and I actually am
one of that crowd. However, I do it slightly different:

void* xmalloc(size_t s) {
void* res = malloc(s);
if(!res) {
fprintf( stderr, "malloc() failed\n");
exit(EXIT_FAILURE);
}
return res;
}

I do this, too, on occasion. Two things to note, though:

1) The strategy of having a low-level routine decide the
fate of the entire program seldom works except for smallish
programs. Once the program gets sophisticated enough to be
maintaining a significant amount of state, it may well want
to do things like writing the internal state to a file before
exiting -- but if xmalloc() or xfopen() or something has
unilaterally decided to exit on its own initiative, Too Bad.

2) The test should be `if (!res && s)'.

 

[Malcolm McLean]

Malcolm McLean wrote, On 19/01/08 13:51:


Then document it as not being a general purpose wrapper. Or better yet fix
it.

xmalloc() will never return null.
Now say that, on a fairly small desktop machine, we want to allocate some
tens of megabytes of memory for a large image. If the machine won't support
an allocation that size, there is no way that xmalloc() can return. It must
terminate the program. Which is almost certainly not what you want, because
it is quite forseeable that user will demand an image larger than you can
handle, and almost certainly you wnat a message such as "image requested too
large, please try a smaller one".

On the othe rhnad if a request for twenty bytes fails, we are in deep
trouble. Quite possibly the only realistic thing to do in such circumstances
is to terminate. That's what xmalloc does by default. Remember, this will
almost never happen - once in a trillion years of operation for the example
you described, under rather different assumptions, just enough for a few
users to experience it occasionally.

It means again that your function is NOT a general purpose malloc wrapper
and not suitable for use by a lot of people.

No. It is suitable for when custom error-handling code adds more cost than
it is worth. Which is often when allocating trivial amounts of memory in
medium-critical applications. It is not for when you have a reasonable
expectation that the memory request will be impossible to fulfil.

 

[Ulrich Eckhardt]

You might be interestwed in xmalloc(), on my website[1], which gets
round this problem of too much error-handling code which will never be
executed.

I took a look at it. Apart from being too complicated for most programs
(yes, some will actually be able to use the additional complexity), it
has one IMHO grave bug: it uses 'int' for the allocation size. Use
size_t, which is the correct type, or do you check if the result of
'strlen()' can be safely converted to an 'int' before calling your
'xmalloc()'?

- sorry, no chocolate for you -

If the amount of memory won't fit into an int you probably shouldn't be
calling the function.

Why not? I would agree if I limited myself to LIP32 systems, but those are
more and more phased out. Even there, several OSs support 3GiB of virtual
address space for userspace, which doesn't fit into said int type.

Also, using int is way harder to make work correctly. Assuming you have some
user input that tells you how many records they want created. You then
simply

unsigned user_input = get_user_input();
int s = user_input * sizeof(record_t);
if(s<0)
size_too_large();

....and you have lost. Newer GCCs will actually be able to see here that two
unsigned values are multiplied, which will create another unsigned value,
so the check whether the result is less than zero is simply omitted! The
simple rationale for this behaviour is that an overflow already causes
undefined behaviour, so all bets are off.

It is for small amounts of memory, and is compatible
with the original C specification of malloc().

IIRC, malloc() uses the following:

void* malloc(size_t);

...which is _not_ compatible.

Also, using a signed value means that we can pick up many bugs.

You created a whole system to catch and even handle malloc errors, including
a user-definable callback. Why not make the largest single allocation a
customisable value too? Then, you could well use size_t for correctness but
catch nonsense values (which depend on the application context) at the same
time.

If garbage
is passed to malloc() then 50% of the time the argument will be negative.
Assuming randomness, if the function is called more than once or twice,
you practically have a guarantee of catching the bug.

Using the above approach, you can forbid e.g. anything beyond 100KiB, which
makes sense in many contexts and will catch way more errors! Seriously, a
customisable check for valid allocation sizes lets you have the cake _AND_
eat it.

Other than that, I'm afraid you won't get around size_t. Everything from
strlen() over malloc() to sizeof work on size_t. Compiling code that mixes
size_t with int is barely possible without proper conversions or lots of
warnings. I would chose neither of those.

Uli