When to check the return value of malloc

[Randy Howard]

Given that your system runs out of memory about every month, how many times
is it likely to need hardware repairs in 25,000 months?

You've raised hand-waving to a new level of art form.

 

[user923005]

Yes, but normally your system will run out of memory when a large amount,
say half a megabyte to hold ten seconds of audio samples, is requested. It
will fail on a request for twenty bytes only once in every 25,000 cases,
assuming both allocation requests are made and the system always fails on
one of them.
Given that your system runs out of memory about every month, how many times
is it likely to need hardware repairs in 25,000 months?


Maybe that should be made clearer. If there is  realistic chance of the
allocation failing, then the failure path should be considered a normal part
of program logic. When the request is for twenty bytes on a syustem with 2GB
installled, however, this becomes less sensible.

What happens if some other application (running at the same time as
your small RAM requestor) succeeds in getting RAM for video editing,
and then you ask for your small packet of RAM?
Not checking the return of malloc() is simply incompetent.
Assuming that something which can fail will always work is not a good
idea, and especially when checking is an ultra-simple operation.
BTW, 99.873% of all statistics about malloc() failure percentage are
made up out of thin air.

 

[CBFalconer]

.... snip ...

/*
failsafe malloc drop-in
Params: sz - amount of memory to allocate
Returns: allocated block (never returns NULL even on block 0)
*/
void *xmalloc(int sz)
{
void *answer = 0;

assert(sz >= 0);
if(sz == 0)
sz = 1;

Why don't you simply make sz a size_t, after which you don't need
the assert (which is a horrible thing to let loose anyhow). If an
int can hold larger values than a size_t you will also avoid asking
for impossibly large allocations, which at present will cause an
infinite loop.

 

[CBFalconer]

Oh? Try the following:

#include <stdio.h>
#include <stdlib.h>

#define SZ 40

int main(void) {
unsigned long count;
void *ptr;

count = 0;
while (ptr = malloc(SZ)) count++;
printf("Failed after %lu tries\n", count);
return 0;
}

Incidentally, on my machine (a 500 Mhz Pentium running W98 FE with
512 meg of memory) this failed after roughly 10 million tries due
to the swap file filling, in about 43 seconds. The interesting
effect is that the swap file had expanded from roughly 10 megs to
about 170 megs. After the program terminated it took several
minutes for the swap file to self (or something) empty. It only
recovered to 15 meg, however.

I have not tested it under any Linux.

--
[mail]: Chuck F (cbfalconer at maineline dot net)
[page]: <http://cbfalconer.home.att.net>
Try the download sectioX-Mozilla-Status: 00091 20:37:25 2008
X-Mozilla-Status: 0801
X-Mozilla-Status2: 00000000
FCC: /C|/Netscape/Users/cbf/mail/sentnews
X-Mozilla-News-Host: free.teranews.com
Message-ID: <[email protected]>
Date: Mon, 21 Jan 2008 20:37:25 -0500
From: CBFalconer <[email protected]>
Reply-To: (e-mail address removed)
Organization: Ched Research http://cbfalconer.home.att.net
X-Mozilla-Draft-Info: internal/draft; vcard=0; receipt=0; uuencode=0; html=0; linewidth=0
X-Mailer: Mozilla 4.75 [en] (Win98; U)
X-Accept-Language: en
MIME-Version: 1.0
Newsgroups: comp.lang.c
Subject: Re: Function Pointers
References: <fda553b2-a24c-4c4b-8e19-4252b55762a9@v67g2000hse.googlegroups.com> <b4bfdec4-2298-42d2-aa84-c5b90873fd8d@s12g2000prg.googlegroups.com> <[email protected]> <[email protected]> <2658fb0d-5824-48e5-9ce5-59dced8dbfb7@m34g2000hsb.googlegroups.com> <[email protected]> <[email protected]> <[email protected]> <[email protected]> <[email protected]>
Content-Type: text/plain; charset=us-ascii
Content-Transfer-Encoding: 7bit

Ben Bacarisse said:
.... snip ...

Oops, so you did. I even read it, actually - but I have a memory
like a... like a... sort of bowl-shaped, you use it to sift, um,
well, cooky stuff, goes in bread...

You are obviously aging. The last few years I keep having problems
remembering the appropriate word (or equivalent). I know I know
it, I just can't dredge it up at the moment. It will probably come
to me later when it is of little use. I am convinced it is an age
effect, but not especially serious, since I can always refer to the
whoozit, or the watch-a-ma-call-it, etc. and anybody of any
intelligence will know exactly what I mean.

 

[Malcolm McLean]

What happens if some other application (running at the same time as
your small RAM requestor) succeeds in getting RAM for video editing,
and then you ask for your small packet of RAM?
Not checking the return of malloc() is simply incompetent.
Assuming that something which can fail will always work is not a good
idea, and especially when checking is an ultra-simple operation.
BTW, 99.873% of all statistics about malloc() failure percentage are
made up out of thin air.

It depends if the video editor has deliberately tailored his request to the
amount of memory in the machine. If he has, all bets are off. If he hasn't,
then the chance of failing on the small packet rather than in the video
editor is quite small.

 

[user923005]

It depends if the video editor has deliberately tailored his request to the
amount of memory in the machine. If he has, all bets are off. If he hasn't,
then the chance of failing on the small packet rather than in the video
editor is quite small.

If you think the odds are quite small that other applications which
are not intentionally trying to allocate all of RAM can make your
malloc() fail are quite small, then I can assure you that you are
mistaken. It is not at all uncommon for people to run lots of
applications at the same time. Sometimes the applications are long-
running applications. Sometimes these applications have resource
leaks. Memory allocation failure for a large or small memory request
is not a rare occurence. I think it is very careless to assume that
any memory allocation will succeed without testing it.

 

[Richard Bos]

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.

That's a massive overestimate, probably because you're going about it
the wrong way. _First_ you ask for the memory, _then_ you start
assigning the values. That way, when malloc() fails, all you have to do
is return an error to the caller.

Typically caller has to simply terminate, or destroy itself and return null
to a higher level, which terminates.

Typically this is not what a wisely written program does. Simply
terminating, throwing all the work up to that point away, is in most
circumstances a bad idea.

Take my current hobby project. It reads in a lot of files with data in
them, and filters out all data which is relevant to it. Then it stores
that data in a tree. I expect all needed data to fit into memory with
room to spare, and I expect never to run out of memory. Nevertheless,
this project _will_ check for memory allocation failure, and should it
occur, the tree constructor will return an error, which will be signaled
by the main program, which will then continue to process as much data as
has already been read and note where it had to stop.
You may think I'm a sucker for writing if-cases and recovery code, but
I'm not. It is actually considerably _easier_ to write the code this way
than to have to cope with randomly aborting programs and then find
another way to process that data. And IME, this is true for the average
program.

Richard

 

[Malcolm McLean]

If you think the odds are quite small that other applications which
are not intentionally trying to allocate all of RAM can make your
malloc() fail are quite small, then I can assure you that you are
mistaken. It is not at all uncommon for people to run lots of
applications at the same time. Sometimes the applications are long-
running applications. Sometimes these applications have resource
leaks. Memory allocation failure for a large or small memory request
is not a rare occurence. I think it is very careless to assume that
any memory allocation will succeed without testing it.

Try to follow the logic.
Memory allocation failure always causes a program to behave in some way that
is sub-optimal for the user. Otherwise the program wouldn't request the
memory at all. So failures, like the Federal Government running out of
money, are rare in terms of cycles. Flash can just about tolerate his
machine running out of memory once a month. I'd say that if the machine runs
out of memory every day, pretty soon the system will be replaced.

The question is, where will the failure occur? In a big allocation, or a
little allocation? With some simplifying assumptions, the answer is very
easy to calculate, and so you can work out the size of allocation that is
orders of magnitude less likely to fail than a hardware failure, that is to
says the error-handling code is vanishingly unlikely to be executed.

If you remove those simplifying assumptions the question does become a bit
more difficult. However the costs of error-handling code are not negligible.
Adding code that will never be executed is classical example of
over-engineering. The answer to the OP's question is "the point at which a
hardware failure becomes statistically more likely".

 

[Richard Heathfield]

Malcolm McLean said:

The question is, where will the failure occur? In a big allocation, or a
little allocation? With some simplifying assumptions, the answer is very
easy to calculate, and so you can work out the size of allocation that is
orders of magnitude less likely to fail than a hardware failure, that is
to says the error-handling code is vanishingly unlikely to be executed.

Here's a function (note - don't believe the comments):

hash *hash_alloc(char *s, int t)
{
hash *p = malloc(sizeof *p); /* hashes are only 10 bytes,
so this won't fail */
p->hv = 0; /* p safely allocated, okay to deref */
while(*s) { p->hv *= 33; p->hv += *s++; }
p->pl = t;
return p;
}

Shove that in a lib. Test it to your heart's content. Clearly, it works.
Now ship the lib binary and a header.

Here's the customer's call:

while(ok && (s = read_a_line(fp)) != EOF)
{
z = hash_alloc(s, i);
if(z == NULL) /* Nice try, customer, but your care is wasted; */
{ /* we've already crashed. */
ok = 0;
}
else
{
++i;
}
}

So whose fault is this? The customer's fault, for running the code on a
fifty-million line log file? That depends on whether the shipped code had
a big notice on the box saying "warning - this lib was written by overly
optimistic developers who haven't properly tested it in a real
environment".

 

[Malcolm McLean]

Malcolm McLean said:

So whose fault is this? The customer's fault, for running the code on a
fifty-million line log file? That depends on whether the shipped code >
had a big notice on the box saying "warning - this lib was written by >
overly optimistic developers who haven't properly tested it in a real
environment".

It depends on whether the code says "returns NULL on failure" or not. If it
does then, clearly, it is lying, and it is the fault of the library vendor.

If it doesn't, customer should consider whether his machine can generate a
potentially infinite supply of them, just as he should consider whether his
arrays will fit on the stack.
When we get the irate phone call, we say "Where did it say the function will
return NULL?" "OK, your code is incorrect. Now what do you want your program
to do if it cannot allocate enough of these hash tables? Terminate with an
error message? That sounds sensible. Now what did it do?"

 

[Richard Heathfield]

Malcolm McLean said:

It depends on whether the code says "returns NULL on failure" or not. If
it does then, clearly, it is lying, and it is the fault of the library
vendor.

Granted. But the important point is this: that the allocation was for a few
lousy bytes, and yet the failure occurred very quickly - it did not take
the many thousands of hours that you were claiming it would take.

A 20 byte allocation doesn't seem like a lot, until you do it in a loop.

 

[Malcolm McLean]

That's a massive overestimate, probably because you're going about > it
the wrong way. _First_ you ask for the memory, _then_ you start
assigning the values. That way, when malloc() fails, all you have to do
is return an error to the caller.

Most structures consist of nested arrays, sizes to be calculated at runtime.

eg

typedef struct
{
float pos[3];
int element;
} Atom;

typdef struct
{
int Natoms;
Atom *atoms;
char id;
}AminoAcid;

typedef struct
{
int N;
AminoAcid *res;
} Protein;

Protein *protein(char *seq)
{
Protein *p;
int i;

p = malloc(sizeof(Protein));
if(!p)
goto error_exit;
p->N = strlen(seq);
p->res = malloc(strlen(seq) * sizeof(AminoAcid));
if(!p->res)
goto error_exit;
for(i=0;i<p->N;i++)
p->res.atoms = 0;
for(i=0;i<p->N;i++)
{
p->res.Natoms = Natoms(seq);
p->res.atoms = malloc(Natoms(seq) * sizeof(Atom));
if(!p->res)
goto error_exit:
fillatomdata(p->res.atoms);
p->res.id = seq;
}
return p;

error_exit:
if(p)
{
if(p->res)
{
for(i=0;i<p->N;i++)
free(p->res.atoms);
free(p->res);
}
free(p):
}
return 0;
}

Protein *protein(char *seq)
{
Protein *p;
int i;

p = xmalloc(sizeof(Protein));
p->N = strlen(seq);
p->res = xmalloc(strlen(seq) * sizeof(AminoAcid))'
for(i=0;i<p->N;i++)
{
p->res.Natoms = Natoms(seq);
p->res.atoms = xmalloc(Natoms(seq) * sizeof(Atom));
fillatomdata(p->res.atoms);
p->res.id = seq;
}
return p;
}

Here are two functions (untested), one written with malloc() and one with
xmalloc(). They are doing a real task, which is to build up a protein from
its amino acid sequence. The geometry code is taken out, essentially all we
are doing is allocating the memory.

 

[Malcolm McLean]

Malcolm McLean said:

Granted. But the important point is this: that the allocation was for a >
few lousy bytes, and yet the failure occurred very quickly - it did not
take the many thousands of hours that you were claiming it would
take.

A 20 byte allocation doesn't seem like a lot, until you do it in a loop.

That's true. You used to get this problem with Midi files - it was extremely
tempting to put the notes into a linked list, but it would run a late
eighties vintage PC out of memory.
Nowadays it isn't a problem, of course, but you can still gobble a lot of
memory with big structures made of small elements.

 

[christian.bau]

So you really are claiming that in some circumstances it's acceptable
to call malloc() and blindly assume that it succeeded, if you've
confirmed that the probability of failure is sufficiently low.

Avoiding checks on the basis that "probability of failure is low
enough" alone is stupid. There is always a chance that someone might
try to attack your software, and artificially create situations where
the probability of failure grows and catches you.

Two situations where you can go without checking for malloc failures:

1. In an embedded system, where you have proof that malloc failure is
not possible. If you can calculate the maximum amount of memory used
(this likely involves examining the code for malloc), then you can
sell a device with enough memory to handle all cases. Sometimes, a
failure of malloc to return memory is unacceptable anyway, whether
detected or not, so you have to give the device enough memory anyway.
In that case, removing the checking code is actually better because it
gives you more leeway in case your calculation was wrong.

2. If your environment is such that any access to null pointers will
crash the program, and a crashing program is harmless: If you want to
calculate a certain result, and write a program for that purpose, and
it needs memory to do the calculation. You will notice that you don't
have enough memory because the program crashes, no need to check for
it. If it crashes because of insufficient memory, you need to run it
on a computer with more memory, or change the code to use less
memory.

So you can remove checking malloc results if (1) you can prove that
malloc cannot fail, or (2) there is nothing that you can do in case of
a failure that is better than crashing, and not checking will result
in a crash as well.

 

[Richard Bos]

That's true. You used to get this problem with Midi files - it was extremely
tempting to put the notes into a linked list, but it would run a late
eighties vintage PC out of memory.
Nowadays it isn't a problem, of course,

Except that nowadays we're not dealing with MIDI any more, but with
MPEG-4. Parkinson' Law trumps Moore's Law.

Richard

 

[Richard Bos]

Most structures consist of nested arrays, sizes to be calculated at runtime.

I can only suppose that one of us must have amazingly unusual tasks to
handle.

Richard

 

[Richard Tobin]

2. If your environment is such that any access to null pointers will
crash the program, and a crashing program is harmless:

You need to be sure that you will access the memory in a way that will
cause a crash. For example, if you allocate a 10,000 element array,
and the first element you access is the 5,000th, then you may not get
an error even if malloc() returned 0.

-- Richard

 

[dj3vande]

2. If your environment is such that any access to null pointers will
crash the program, and a crashing program is harmless:

I would say that a properly written wrapper that ends up calling
exit(EXIT_FAILURE) (or even abort()) on allocation failure is a better
solution in this case than letting the null pointer dereference crash
the program.


dave

 

[Richard Harter]

On Tue, 22 Jan 2008 17:20:13 +0000 (UTC),

I would say that a properly written wrapper that ends up calling
exit(EXIT_FAILURE) (or even abort()) on allocation failure is a better
solution in this case than letting the null pointer dereference crash
the program.

Just so. Even better is to make provision for callbacks to
handle cases where something useful can be done. Who would think
of doing such a thing.

 

[Flash Gordon]

Richard Bos wrote, On 22/01/08 15:30:

I can only suppose that one of us must have amazingly unusual tasks to
handle.

I agree with Richard here.