When to check the return value of malloc

[Malcolm McLean]

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.

If you are caling malloc() then usually it means that the state is currently
in a corrupt or partially constructed situation. So you have to handle that
on the dump. Which means that the whole program has to be written in a very
careful, sophisticated manner.

 

[Ulrich Eckhardt]

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.

Yes, true. This code is definitely not general purpose library code! In fact
a library should never invoke exit() on its own but always signal the error
to the calling code.

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

Hmm, maybe. Technically you are right, as the behaviour would better match
that of malloc(). Practically though, I can't remember a case where I
actually used a zero allocation size for anything sensible. To be honest, I
don't even know what the use of a zero-size allocation would be.

Uli

 

[Malcolm McLean]

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

I'd want

int user_input = get_user_input();
/* if I was sensible, sanity check here, but this is a sloppy program */
int s = user_input * sizeof(record_t);
/* at this point we've got an overflow, so compiler issues "arithmetic
overflow" and terminates. */

Unfortunately we don't get this yet. Almost no compilers will handle oveflow
nicely. However if the result is negative, which is a sporting chance, at
least we can pick it up in the call to xmalloc(). If we are in a loop with
essentially random values being typed in by user, the chance of a negative
becomes statistically certain.

 

[Flash Gordon]

Malcolm McLean wrote, On 19/01/08 15:22:

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 HAPPENS REGULARLY!

Just because you don't stress your machine doesn't mean others don't.
For those of us that make machine work THEY RUN OUT OF MEMORY. When
there is NO memory left, that mean there is NO memory left. That HAPPENS
REGULARLY and means that REGULARLY malloc FAILS.

Can you understand that now? I REGULARLY DO NOT HAVE THE FREE MEMORY TO
ALLOCATE ONE BYTE.

At least in this thread you have stopped recommending simply ignoring
out of memory conditions, which was where I started pointing out how bad
your advice was.

No. It is suitable for when custom error-handling code adds more cost

<snip>

No, it is not suitable for anyone that does not want YOUR arbitrarily
introduced restrictions.

 

[Richard Harter]

[snip original query]

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;
}

This is the basis for the approach that I take; however as Eric
notes this form is not entirely satisfactory for large programs.
The problem (in my view) lies in the error handling process.
Commonly neither the wrapper nor the using code is in any
position to make an intelligent and useful response to this kind
of resource failure. I find it better to create and use (and
reuse) a general purpose error handler that extracts state
information from the program, writes it to an error file, and
then exits gracefully.

I take the view that allocator failure is a reflection of a
program bug, commonly because there are memory leaks, there is a
size computation error, or the scale of the problem is larger
than the systems size. Such bugs are often not easily
reproducible and can occur in remote sites. I want all the
information I can get and I want it in an intelligible form.

My experience is that the combination of a portable instrumented
wrapper for allocation/deallocation and a portable error handler
that generates detailed error files works quite well and is worth
the investment up front for programs of any size.

Of course, this isn't universally true; there are environments
where a program can and even must do something intelligent about
an allocation failure and limp on.

 

[gw7rib]

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.

As others have said, I'd recommend always testing.

But - if the 20 bytes is only needed temporarily, and will be finished
with before you next need 20 bytes - why not just allocate one buffer
at the beginning of the program and use that each time? That way you
would only need one malloc - or, for such a small size, use an array.

Hope this helps.
Paul.

 

[James Kuyper]

Ulrich Eckhardt wrote:
....

that of malloc(). Practically though, I can't remember a case where I
actually used a zero allocation size for anything sensible. To be honest, I
don't even know what the use of a zero-size allocation would be.

In some cases, you want to allocate n bytes, where n is determined at
run-time, and can be 0. An obvious approach would be to not do any
allocation when n is 0. However, in some circumstances it can greatly
simplify your code simply to allocate n bytes, regardless of what n is.
Loops that work with the allocated memory can often be easily written to
handle n==0 correctly without needing any special case logic:

for(i=0; i<n; i++) ...

Of course, this gets messed up by the fact that the standard provides
two possibilities; if your code has to work correctly with either
possibility, the advantage pretty much disappears. However, if you can
afford to restrict your code to platforms where malloc(0) returns a
non-null pointer, it can sometimes simplify your code to rely upon that
fact. I've never done so - portability is too important to me.

 

[CBFalconer]

Ulrich Eckhardt wrote:
.... snip ...


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

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

That is more easily handled by:

void *xmalloc(size_t s) {
void *res;

if (!s) s++;
if (!(res = malloc(s))) {
fputs("malloc() failed\n", stderr);
exit(EXIT_FAILURE); /* optional */
}
return res;
}

which can leave the exit decision to the caller. It also makes the
return of NULL a positive indication of failure.

 

[Eric Sosman]

If you are caling malloc() then usually it means that the state is
currently in a corrupt or partially constructed situation. So you have
to handle that on the dump. Which means that the whole program has to be
written in a very careful, sophisticated manner.

Writing carefully is a Good Thing, certainly. But I take
issue with the notion that doing so is "sophisticated;" going
further back, I reject the "corrupt" piece. Observe that the
memory that wasn't allocated is not yet linked into any of the
program's data structures (how could it be?), so the state
dumper or state repairer or whatever will not encounter it. If
you're extremely sloppy there might be an uninitialized pointer
somewhere, but that's the fault of the programmer and not of the
malloc() failure. More typically there's just a NULL somewhere
in the data structure, and the state massager knows enough (ought
to know enough) not to follow NULLs.

 

[Malcolm McLean]

Writing carefully is a Good Thing, certainly. But I take
issue with the notion that doing so is "sophisticated;" going
further back, I reject the "corrupt" piece. Observe that the
memory that wasn't allocated is not yet linked into any of the
program's data structures (how could it be?), so the state
dumper or state repairer or whatever will not encounter it. If
you're extremely sloppy there might be an uninitialized pointer
somewhere, but that's the fault of the programmer and not of the
malloc() failure. More typically there's just a NULL somewhere
in the data structure, and the state massager knows enough (ought
to know enough) not to follow NULLs.

Say we've got this, not too contrived situation

typedef struct
{
int Nemployees;
char **names;
float *salaries;
} Workforce;

when constructing a Workforce, we set up the names array correctly, and
malloc() fails on the employees.
We've now got to be pretty careful. Nemployees is needed to destroy the
names correctly, but salaries is null.

It can be negotiated successfully, but remember how hard this code is to
test. We've got to test for several patterns of allocations failing and the
recovery code saving the state properly and destroying the object properly.
It is also very easy to forget to initialise the salaries member to null -
you're intialising it on the malloc, after all. However should a name fail
to allocate, you're left with a garbage pointer.

 

[Randy Howard]

If you are caling malloc() then usually it means that the state is currently
in a corrupt or partially constructed situation. So you have to handle that
on the dump. Which means that the whole program has to be written in a very
careful, sophisticated manner.

BTW, they xmalloc() you referenced from your website earlier uses int
instead of size_t for the size. Is that intentional? If so, why?

 

[Randy Howard]

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().

I asked a similar question before I read this article. Feel free to
ignore the other one, I'd cancel it, but that never works anyway.

I completely disagree with the above, btw.

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.

No, if a very large (and legal) request is passed, you'll think it's
garbage when it is not. That's not a helper function, that's a
disaster.

 

[Randy Howard]

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".

No, it means that the wrapper is unsuitable for such programs. If you
attempt to malloc 1800MB, and due to process address space already
consumed, 1735MB is the most you can get on platform W for example,
then you can have the higher level calling code get the NULL, check for
it, then ask for a smaller size instead, if that makes sense for a
given app. This may involve no message to the user at all and be done
internally.

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.

Or free() or realloc() some other block that could be made smaller, say
some internal cache that doesn't have to be so large if memory
resources become critical. Or even wait briefly and retry before
giving up. This is the problem with "one size fits all" solutions.

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.

People have malloc fail on them quite often nowadays with real-world
app bloat sizes on 32-bit platforms. You could argue that 64-bit
adoption would have been far slower, and things like /PAE on win32
wouldn't exist otherwise.

No. It is suitable for when custom error-handling code adds more cost than
it is worth.

It could be made suitable for such things with a few changes, like
using the proper type for the size parameter.

 

[Keith Thompson]

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.

[...]

How difficult is it to compute the probability that a single call to
malloc() will fail? How difficult is it to check the result? What
are the "maintenance costs" of ensuring that your probability
estimates remain correct as the program evolves? Compare and
contrast.

Just check the result of every single call to malloc(). If you can't
think of a more sensible way to handle failure, just abort the
program. Use a wrapper if you like; call it "malloc_or_die()". There
is no excuse for not checking the result of malloc(), except *maybe*
in a quick throwaway program.

(Please note that I'm not generally advocating aborting the program on
a malloc() failure; I'm just saying that it's better than ignoring
it.)

 

[Malcolm McLean]

BTW, they xmalloc() you referenced from your website earlier uses int
instead of size_t for the size. Is that intentional? If so, why?

Intentional. To emphasise that thje function is intended for small
allocations that cannot fail, for some value of "cannot".
If you expect the allocation to fail, you shouldn't be calling xmalloc(),
because its strategy is to repeatedly nag until memory becomes available
(assuming you don't terminate). If the memory simply isn't avialable, this
isn't very sensible.

 

[Malcolm McLean]

How difficult is it to compute the probability that a single call to
malloc() will fail? How difficult is it to check the result? What
are the "maintenance costs" of ensuring that your probability
estimates remain correct as the program evolves? Compare and
contrast.

Just check the result of every single call to malloc(). If you can't
think of a more sensible way to handle failure, just abort the
program. Use a wrapper if you like; call it "malloc_or_die()". There
is no excuse for not checking the result of malloc(), except *maybe*
in a quick throwaway program.

(Please note that I'm not generally advocating aborting the program on
a malloc() failure; I'm just saying that it's better than ignoring
it.)

Typically something between 33% to 50% of code will be there to handle
malloc() failures, in constructor-like C functions. If malloc() fails they
destroy the partially-constructed object and return null to caller.
Typically caller has to simply terminate, or destroy itself and return null
to a higher level, which terminates.

So xmalloc() is a worthwhile simplifier. But it is not a good strategy for
dealing with out of memory conditions. It's simply the best strategy that I
can think of that can be implemented in conforming C. When the
error-handling code is called only once in a trillion years or so of
operation, obviously we are not too bothered about ideal operation. Howevewr
you cannot be expected to do this calaculation all the time. The heuristic
is "is this aloocation likely to be large enough to have a realistic chance
of failure?" If the answer is yes, call malloc(), check the result, and
treat null as one o fthe normal execution paths of the program. Otherwise
call xmalloc(), and just tolerate the sub-optimal processing.

 

[Randy Howard]

Intentional. To emphasise that thje function is intended for small
allocations that cannot fail, for some value of "cannot".

I never assume that a given malloc can not fail.

If you expect the allocation to fail, you shouldn't be calling xmalloc(),

Your version, I agree completely.

because its strategy is to repeatedly nag until memory becomes available
(assuming you don't terminate). If the memory simply isn't avialable, this
isn't very sensible.

It's also a strategy that will rarely provide useful results.

 

[Malcolm McLean]

It's also a strategy that will rarely provide useful results.

I don't think that's true. The system fails to deliver 20 bytes. A window
pops up on screen saying "program Fred needs more memory to continue, please
shut down something else". User shuts down his Internet Explorer, which is
in the background doing nothing much, and program Fred continues.

I know that modern OSes tend to warn the user long before this happens, and
Linux has a dangerous policy of overcomitment. But the basic idea is
reasonable.

Then xmalloc() is by default a "malloc or die". Which has a kind of utility.
If you want "malloc or save and die", you just pass in the save function.

If you've got better semantics for xmalloc, then by all means share them. I
don't think it's an ideal solution at all. It's the least bad solution to
the problem of trivial requests failing I've been able to devise so far.

 

[Eric Sosman]

Typically something between 33% to 50% of code will be there to handle
malloc() failures, in constructor-like C functions.

Please cite the research that measured this surprising figure.

 

[Malcolm McLean]

Please cite the research that measured this surprising figure.

Just check my website.
Virtually all the files are written in "object style". That is to say, there
is a function with the same name as the public structure, though in lower
case, and a corresponding kill function. There are also opaque method
functions which manipulate the structures.

Count the number of lines that would not be needed if malloc() could be
guaranteed never to fail. The answer is about 33%-50%, within the
constructor functions. You've got to check the allocation; mostly I reduce
complexity by adding a goto which then calls the kill function. But even so
nulls need to be put in so that the kill function doesn't try to pass
garbage pointers to free().