When to check the return value of malloc

[CBFalconer]

getc and fgetc will use the same stdio buffer, so if the problem
is buffer size that won't solve anything.

You miss the point, which is that getc is encouraged to be a macro,
so that use can avoid any system function calls. When properly
implemented it effectively puts the system buffers in the program
coding.

 

[CBFalconer]

[snips]

I will never understand this bizarre reliance on assert, when
the language actually contains conditional branching constructs.

I suppose that use if/else is more work that an assert().

The voice of someone who never examines the generated code.

 

[pete]

santosh wrote:

I think I get what Malcolm is getting at.
Basically he does not want to
implement a full error-detection and recovery scenario for small
allocations, which /he/ thinks have only a minute chance of failure.
For such allocations he uses xmalloc() which will either get you the
requested memory or call exit().

I don't see a relationship between
the size of the requested memory,
and whether or not quitting the program
is the best way to handle an allocation failure.

 

[user923005]

I don't see a relationship between
the size of the requested memory,
and whether or not quitting the program
is the best way to handle an allocation failure.

Neither is there any relationship between the size of the requested
block and the probability of failure:

Allocation series 1:
void *bigarr[10000000];
size_t index;
for (index = 0; index < sizeof bigarr/sizeof bigarr[0]; index++)
{
bigarr[index] = malloc(10);
}

Allocation series 2:
void *p = malloc(10000000);

as far as I can see, allocation series 1 never allocated a large
object and yet is more likely to fail than allocation 2 which does
allocate a large block of RAM.

 

[Syren Baran]

Howdy,

I was reflecting recently on malloc.

Had a somewhat heated discussion on that recently myself.

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.

Actually its not as much about the size but more about the "what else".
If you have an alternative to malloc'ing memory, nice, check and do it.
If you cant continue, do a gracefull exit, e.g.
in standard C i would wrap malloc, do or die style,
in Unix style systems i would catch SIGSEGV.

Dont know if anyone pointed this out, but alternatives to malloc are no
more safe, e.g. instead of
somefunc(){
char* buffer;
buffer=malloc(SIZE); //oh, i need to check?
//do something
free(buffer);
}

somefunc(){
char buffer[SIZE]; //how do i check here?
// do something
}

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?

I think the basic rule is tied to the question "can i do something
better than quiting?"
I dont think you can find a real world solution to this question in the
C standard specs. It just too OS specific.

 

[Ian Collins]

Like when you try to open the app's configuration file and it doesn't
exist, or isn't readable, at which point you have many options: log the
reason for the failure to the system log and exit. Pop something up to
alert the user, who can then either fix the problem and continue or
decide to cancel the app run. Create a default config file, alert the
user (or log, or both) and continue or exit. Skip the file entirely, use
defaults from the app, notify the user (and/or log) and continue or exit.

No, that isn't an appropriate condition for an assert.

 

[Richard Bos]

I suppose that use if/else is more work that an assert().

Some kinds of laziness are a good trait in a hacker. Some kinds are a
bad trait. This kind of laziness is definitely in the latter group. Do
it _right_, damn it. In the end, it will save you effort. A hacker
should know the difference between short-term laziness and long-term
laziness.

Richard

 

[Kelsey Bjarnason]

[snips]

Let me provide a counter example, I had an embedded product in the
field and we received a number of reports from customers that the
units were
rebooting. When we checked the assertion log of one, we found a
device was generating an interrupt when it should not, which would
have cased bad data to enter the system.

And this is impossible to do with an if (...) which wouldn't abort the
program, but instead give you endless opportunities to do other, more
graceful things - or to simply abort, should that be best?

I will never understand this bizarre reliance on assert, when the
language actually contains conditional branching constructs.

I suppose that use if/else is more work that an assert().

Is it?

When I'm writing code, I tend to write it to deal with the failure
conditions it is going to have to cope with anyways, so I don't see how
it's any more effort to do what I'm already doing in the first place.

What shall I assert on? If x has a value of 2, and I add 1 to it, should
I assert it has a value of 3? No, that's silly. How about, oh, a file
open failure? No, in many cases that is a predictable - and manageable -
condition, assert is the wrong animal.

Hmm. How about, oh, the "update" function in a DB client, where the
update function expects to have an established connection to the server?
No, if I'm doing an update, it means I have "live" data, aborting at this
point would be a most unacceptable thing to do.

Let's see what some other folks have used assert for, right here in this
group.

size_t myStrLen(char const* s)
{
assert( s != NULL );
return strlen (s);
}

Nope, no good. Falls on its face in the presence of NDEBUG.

In another post:

C doesn't allow arrays to be assigned directly. You can use
memcpy:
assert(sizeof my_orders[0].my_mink == sizeof milk);
memcpy(my_orders[0].my_milk, milk, sizeof milk);

Err... it's okay to memcpy if the objects are of incompatible sizes, as
long as NDEBUG is defined? Why?


Another:

I consider this better:
assert(NULL != s);
before calling strlen().

Err... why? Again, the test silently and magically goes away, leaving
the code open to passing a NULL to strlen, if NDEBUG is defined, where a
proper if() test would not silently and magically go away, but continue
handling the case regardless.

This, BTW, led to the comment that assert should never appear in
production code - which, if we apply the thinking, completely invalidates
the use of the assert in the first place. If we risk passing a NULL
where we don't want it, and we're relying on assert to check it, and we
disable assert in the production build, we *also* disable the test for
the NULL - in the production code. This is good practise?


Another:

I put assert() in to check for bugs in my program. For example, if a
certain computation should always produce a number between 0 and N, I
might put in an assert() to verify this. What would be the point of
removing the test in the final version of the program?

No, there wouldn't be any point in removing it... and almost as little
point in simply aborting, when any number of recovery strategies may be
possible, ones which don't simply discard the user's data. Of course,
"what would be the point of removing this in the final version"
implicitly suggests the reliance on the assert "doing its thing" even in
production builds, which is all well and good until your code is compiled
by someone else who thinks asserts should not be in production builds, at
which point all your wonderful error handling just instantly ceased to
exist and your code now pukes and dies on stuff it *should* have handled
gracefully, and would have if you weren't relying on assert as an error
handler.


In case after case of code I see here in CLC, use of assert leads
*directly* to code which fails, miserably, in unpredictable ways, because
it relies on assert to handle errors, rather than using a sensible error
handling strategy. Totally aside from the whole notion that the best way
to handle an error is to abort the app, a notion I find absurd in the
extreme, we're still left with the fact that the whole concept only works
if you can guarantee your code *never* ends up in another build
environment, where the developer using it prefers not to include asserts
in production builds. Poof, there goes all your "error checking".

Yeah, fine, it's more work to write an if statement and handle the
failure condition. Granted. If ease is the excuse, though, we can also
give up all error handling. Just assume files open, allocations work,
network connections succeed, users enter properly formatted data of the
correct length. It's easier, after all, to not bother writing code to
test such things.

Or perhaps the notion is "easy error handling", in which case we can, oh,
simply assert() after a call to fopen, since the user will always type
file names correctly and have appropriate permissions, therefore any
failure to open the file must be a case for the application dying.

It's certainly easier than, say, asking the user for another file name,
or logging that the file couldn't be found or that he lacks permissions
to access it.

In fact, in the (admittedly non-exhaustive) searching I've done, the only
case I've seen so far where assert has any justification in existing is
the following: assert ( INT_MAX >= UCHAR_MAX ).

Mind you, that said, this strikes me as something to test at compile
time, not run time.

Is it easier, is it less work? Sure. Does that necessitate it being a
good thing? Not in the slightest. Most of the uses of assert I see
around here, I wouldn't have on toast.

 

[Kelsey Bjarnason]

[snips]

The reason is that if the system won't give to 100MB for that massive
video sequence, it is perfectly forseeable.

And, being forseeable, I've very likely already got a strategy in place
that says "If I can't get 100MB, I might be able to get 50MB. Won't be
quite as efficient, but may be perfectly usable." Oh wait, you can't,
your allocator just crashed your app.

You ought to tell the user
to buy a better machine or upload a shorter clip.

Or allocate a smaller buffer and process it in stages. Oh wait, you
can't, your allocator just crashed your app.

If it won't 100 bytes
for the name of the file to hold that video sequence, we're in real
trouble.

Not necessarily. Again, you assume a lot of things which are simply not
true of necessity.

If I can't allocate 100 bytes, it may well be because I've already
allocated 100MB and there's nothing left to allocate. I _could_ possibly
reduce my buffer size to 50MB, freeing up space for the 100 byte
allocation. Oh wait, no, I can't, the allocator just crashed my app.

Whilst we could unwind our function and try to continue, the
chance of the next allocation failing is very high.

Why? I just freed 50MB to be used by, at an off-the-cuff estimate, some
half a million of those 100 byte allocations. Oh, wait, I *would* have
done that, but the allocator crashed the app.

xmalloc() allows you to put that decision into the hands of the user

Since we're talking video here... if my application has three videos
open, with edits, and the user tries to open a fourth, but runs out of
memory, *I* would probably tell the user that: "Sorry, file's too big,
can't allocate memory for it. Select another file, or select "cancel" to
return to your editing."

Oops, your method doesn't allow that; his options are to get the memory,
or lose all his edits. Yeah, good job. I'm sure he'll be thrilled.

or
to execute emergency saves.

Nice trick. Explain to us how you're going to "execute emergency saves"
of data in buffers where the only reference to them is a pointer local to
another function? Whoops, you can't.

Besides, why "emergency"? Cancelling the load operation and going back
to work on the three files he's already got open is a perfectly valid
option - just one *you* don't allow, preferring to send all his data down
the toilet.

The question which is hard to answer is what to do when the allocation
consists of a huge number of tiny requests

The same thing you do for a small number of huge requests: whatever is
appropriate, which is virtually *never* simply killing the app without so
much as a chance to save the data.

 

[santosh]

I don't see a relationship between
the size of the requested memory,
and whether or not quitting the program
is the best way to handle an allocation failure.

Malcolm has subsequently explained in another thread with Kelsey
Bjarnason that /he/ considers quitting an acceptable, even good,
strategy for failures of small sized allocations, like say a few
hundred bytes. He has said more than once that his xmalloc() is /not/
suitable for use if there is any chance of the allocation failing at
all, i.e., almost never.

 

[Malcolm McLean]

Malcolm has subsequently explained in another thread with Kelsey
Bjarnason that /he/ considers quitting an acceptable, even good,
strategy for failures of small sized allocations, like say a few
hundred bytes. He has said more than once that his xmalloc() is /not/
suitable for use if there is any chance of the allocation failing at
all, i.e., almost never.

The reason is that if the system won't give to 100MB for that massive video
sequence, it is perfectly forseeable. You ought to tell the user to buy a
better machine or upload a shorter clip. If it won't 100 bytes for the name
of the file to hold that video sequence, we're in real trouble. Whilst we
could unwind our function and try to continue, the chance of the next
allocation failing is very high. If we are built on top of unstable library
functions, the chance of a library function failing is now very high. And a
failure of 100 bytes is unlikely to happen. So quitting is not too bad,
probably it is the only thing that can be done, realistically.

xmalloc() allows you to put that decision into the hands of the user, or to
execute emergency saves. It doesn't allow you to shrink the allocation and
apply an alternative algorithm.

The question which is hard to answer is what to do when the allocation
consists of a huge number of tiny requests, for instance to build a big
tree. You've still run the machine out of memory. However the allocation is
no longer vanishingly unlikely to fail, and by destroying the tree you might
recover enough memory to continue.

 

[santosh]

The reason is that if the system won't give to 100MB for that massive
video sequence, it is perfectly forseeable. You ought to tell the user
to buy a better machine or upload a shorter clip.

Or perhaps use a less memory hungry algorithm, or request the operator
to manually free up some memory.

If it won't 100
bytes for the name of the file to hold that video sequence, we're in
real trouble.

Not necessarily. If a message can be sent to the user, then he might be
able to bring the system on track, by say, temporarily killing some
other application etc. Or we might be able to free up some memory in
our program that's not currently strictly necessary and then try for
those hundred bytes again.

A well designed program might even have a small pool of memory reserved
for such emergency purposes, perhaps just enough to log a message or
something.

Whilst we could unwind our function and try to continue,
the chance of the next allocation failing is very high.

As we unwind, a good OS could recover some memory from the freed stack
space.

If we are
built on top of unstable library functions, the chance of a library
function failing is now very high.

Yes. But this has nothing to do with our program. Our program must still
make the best effort possible, even if it's environment is fragile.

[...]

xmalloc() allows you to put that decision into the hands of the user,
or to execute emergency saves. It doesn't allow you to shrink the
allocation and apply an alternative algorithm.

Yes, that's my objection. Sometimes shrinking the allocation size might
be the best strategy. Basically xmalloc() is dictating high-level
policy when it is merely a primitive.

The question which is hard to answer is what to do when the allocation
consists of a huge number of tiny requests, for instance to build a
big tree. You've still run the machine out of memory. However the
allocation is no longer vanishingly unlikely to fail, and by
destroying the tree you might recover enough memory to continue.

Or you might be able to save the partial tree and exit.

 

[Kelsey Bjarnason]

[snips]

Let's consider (for a moment) small allocations. We have one initial
question to answer:
"Why was malloc(50) called instead of just char foo[50]?"

Possibility: the app uses something akin to a linked list to store the
actual file names loaded, presumably so it can later save them. The file
select and open mechanism can thus open the file _and_ add the name of
the file to the list. At least as applies to the example Keith suggested.

 

[Kelsey Bjarnason]

[snips]

I wonder if this use of the word "app" betrays a particular outlook?
It's not a term I would use to describe any of my programs. Or is "app"
just your word for a program? Is "grep" an app?

Personally, I tend to divide programs into a couple different categories,
although this is somewhat less than formally defined.

A tool or utility would be anything from zip to sort to grep to tar.
An app is generally an interactive program, such as a word processor.
A server is, well, a server.

Suppose a test in grep's regular expression compiler determines that
there's a bug in that compiler. What should grep do? Attempt to
somehow preserve the user's data? That doesn't make sense: grep isn't
modifying any data.

No, and tools such as this I'd tend to suggest need to be somewhat less
fanatical.

Put it this way: it is unlikely in the extreme that if grep fails, the
data I've been putting together for the last two hours is going to go
*poof*. If my word processor falls on its face and dies, things may be a
little different.

Continue regardless, perhaps triggering a
segmentation fault with no clue as to the cause, or perhaps producing
incorrect output, so that my shell script deletes the wrong files? No,
the only sensible thing to do is abort, with a message indicating that
there's a bug in the program, and an exit status that tells the caller
that it didn't work. Which is what assert() does.

How'd you determine that there was a bug? Oh, right - by testing some
condition, one which should not occur. Sounds good. So you send me a
copy of this, which I incorporate into my own tool chest.

Here's the problem: you rely on assert to do your error handling. I
build with NDEBUG defined, as a rule, for "production" builds.

Oops. All your wonderful error handling code just magically disappeared,
the code *does* produce invalid results, incorrect files *do* get
deleted. Why? Because you used the worst possible method for detecting
problems, one which simply ceases to function _at all_ in what I consider
a production build.

If you used a proper validation, one which doesn't magically disappear
simply because I disable asserts in my builds, the code would still work,
still do the right thing.

So, in short, you've given another example of precisely why one should
*not* be using assert.

 

[Kelsey Bjarnason]

What is release mode? Just about every product I've worked on had it's
own set of compiler options used for production builds.

A number of popular compilers, in the DOS days, at least, defaulted to
two basic modes of operation. One was "debug mode", where stack frames
were generated, symbols injected, optimizations turned off and so forth,
making it easier to debug the app.

The other mode was the non-debug mode with a default set of optimizations
turned on, symbols not injected (or stripped, as the case may be), stack
frames created only where necessary... and in many cases, NDEBUG defined.

Such a mode is often referred to as "release mode", as in "the mode other
than the one used to debug the app, the mode used when you plan to ship
or use the app, i.e., release it."

 

[Malcolm McLean]

If I can't allocate 100 bytes, it may well be because I've already
allocated 100MB and there's nothing left to allocate. I _could_ possibly
reduce my buffer size to 50MB, freeing up space for the 100 byte
allocation. Oh wait, no, I can't, the allocator just crashed my app.

That's where your intuition fails you.
if you successfully allocate 100MB, it is inconceivable that a request for
100 bytes should then fail, unless the big allocation was deliberately
tailored to the amount of memory in the machine.

You could reduce a large allocation, change algorithms, and then get those
100 bytes. xmalloc() would not prevent you from doing that, because the
handler function can shrink the 100MB buffer down to 50 MB. There's even a
hook pointer provided to help. But why do I get the impression that people
are fantasisiing about these amazing recovery strategies?

 

[Richard Tobin]

I'm not going to comment on most of this article, since I don't have the
time or inclination, but there's one point that caught my attention:

Totally aside from the whole notion that the best way
to handle an error is to abort the app, a notion I find absurd in the
extreme

I wonder if this use of the word "app" betrays a particular outlook?
It's not a term I would use to describe any of my programs. Or is
"app" just your word for a program? Is "grep" an app?

Suppose a test in grep's regular expression compiler determines that
there's a bug in that compiler. What should grep do? Attempt to
somehow preserve the user's data? That doesn't make sense: grep isn't
modifying any data. Continue regardless, perhaps triggering a
segmentation fault with no clue as to the cause, or perhaps producing
incorrect output, so that my shell script deletes the wrong files?
No, the only sensible thing to do is abort, with a message indicating
that there's a bug in the program, and an exit status that tells the
caller that it didn't work. Which is what assert() does.

-- Richard

 

[santosh]

That's where your intuition fails you.
if you successfully allocate 100MB, it is inconceivable that a request
for 100 bytes should then fail,

Why inconceivable? Lets say I have 512 Mb. The program in question has
allocated and is using 100 Mb. I also have four other programs, each
using roughly 100 Mb. Now it's perfectly likely that the next
allocation request my program makes, even a 100 byte one will fail,
because the system memory is nearly all used up and the rest is
occupied by the OS itself.

unless the big allocation was
deliberately tailored to the amount of memory in the machine.

This is also one scenario which a good program must handle.

You could reduce a large allocation, change algorithms, and then get
those 100 bytes. xmalloc() would not prevent you from doing that,
because the handler function can shrink the 100MB buffer down to 50
MB. There's even a hook pointer provided to help. But why do I get the
impression that people are fantasisiing about these amazing recovery
strategies?

It's not fantasy. It is done, but I'll admit that only rarely, because
it requires more than a little technical competence and patience. While
commercial code is often hamstrung by tight schedules, hobby code can
suffer because the author lacked the necessary patience.

But it's not fantasy. And xmalloc() is not the proper primitive for the
job.

 

[Keith Thompson]

That's where your intuition fails you.
if you successfully allocate 100MB, it is inconceivable that a request
for 100 bytes should then fail, unless the big allocation was
deliberately tailored to the amount of memory in the machine.

"I do not think that word means what you think it means."

That's where your imagination fails you. *Any* call to malloc() can
fail.

What if the allocation *was* deliberately tailored to the amount of
memory in the machine? What if the allocation is tailored to the size
of an external file, and an attacker constructs a file for the purpose
of crashing your application, knowing how sloppy you are about
checking the result of malloc()? What if the underlying memory
allocator uses different pools for different allocation sizes, and the
pool for small allocations has just run out? What if 100MB just
happens, by coincidence, to be within 100 bytes of the amount of
memory available (not likely in any one run of the program, but it
*will* happen eventually).

You could reduce a large allocation, change algorithms, and then get
those 100 bytes. xmalloc() would not prevent you from doing that,
because the handler function can shrink the 100MB buffer down to 50
MB. There's even a hook pointer provided to help. But why do I get the
impression that people are fantasisiing about these amazing recovery
strategies?

Suppose an interactive application has a routine to open and load a
file of a particular type. It needs to allocate space for the
contents of the file itself, for the name of the file, and for some
other associated information. Let's say it calls malloc() 3 times,
once for the 100MB needed for the file, once for the 50 bytes needed
to store a copy of the file name, and once for the 27-kilobyte index
it constructs so it can access the information more efficiently. If
any one of those allocations fails, it can free() everything it's
allocated, present an error message to the user, and return to the
caller, allowing the user to continue working on other files.

What an amazing recovery strategy!

 

[Eric Sosman]

If I can't allocate 100 bytes, it may well be because I've already
allocated 100MB and there's nothing left to allocate. I _could_ possibly
reduce my buffer size to 50MB, freeing up space for the 100 byte
allocation. Oh wait, no, I can't, the allocator just crashed my app.

That's where your intuition fails you.
if you successfully allocate 100MB, it is inconceivable that a request
for 100 bytes should then fail, [...]

I'll have to explain this phenomenon to my bank: If I write
a big check and they honor it, they are then obliged to honor a
subsequent small check, even if the first one emptied my account.
Sweet!