When to check the return value of malloc

[sandeep]

Hello friends~~

Think about malloc.

Obviously, for tiny allocations like 20 bytes to strcpy a filename,
there's no point putting in a check on the return value of malloc... if
there is so little memory then stack allocations will also be failing and
your program will be dead.

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

So somewhere in between there must be a point where you stop ignoring the
return value, and start checking it. Where do you draw this line? It must
depend on whether you will deploy to a low memory or high memory
environment... but is there a good rule?

Regards~~

 

[Tim Harig]

Obviously, for tiny allocations like 20 bytes to strcpy a filename,
there's no point putting in a check on the return value of malloc... if
there is so little memory then stack allocations will also be failing and
your program will be dead.

Always check for error conditions. The heap and stack use different areas
of memory; and, the stack memory is likely already be allocated. Therefore, it
is quite possible that you still have stack memory left while you cannot
request any more from the heap.

 

[Tim Harig]

Always check for error conditions. The heap and stack use different areas
of memory; and, the stack memory is likely already be allocated. Therefore, it
is quite possible that you still have stack memory left while you cannot
request any more from the heap.

BTW, if you are wondering how you can possibly go on without any more heap
memory, most users prefer a message telling them that you cannot perform an
operation because of insufficient memory then simply having the program
crash with a segfault. It also makes your troubleshooting and debugging
much easier.

 

[bart.c]

Hello friends~~

Think about malloc.

Obviously, for tiny allocations like 20 bytes to strcpy a filename,
there's no point putting in a check on the return value of malloc... if
there is so little memory then stack allocations will also be failing and
your program will be dead.

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

So somewhere in between there must be a point where you stop ignoring the
return value, and start checking it. Where do you draw this line? It must
depend on whether you will deploy to a low memory or high memory
environment... but is there a good rule?

For a proper application, especially to be run by someone else on their own
machine, then you should check allocations of any size (and have the
machinery in place to deal with failures sensibly).

For anything else, where you don't expect a failure, or it is not a big
deal, then use a wrapper function around malloc(). That wrapper will itself
check, and abort in the unlikely event of a memory failure. But it means you
don't have to bother with it in your main code.

It is also possible to just call malloc() and assume it has worked. Your
experience will tell you when you can get away with that. But only do that
with your own programs...

 

[Eric Sosman]

Hello friends~~

Think about malloc.

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

Obviously, you are ignorant of the Sixth Commandment. The text
of all ten Commandments, along with learned commentary can be found

if
there is so little memory then stack allocations will also be failing and
your program will be dead.

On many systems -- maybe even on most -- memory for auto variables
and memory for malloc() is drawn from different "pools," and one can
run out while the other still has ample space.

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

Yes, you should definitely check for a NULL return.

So somewhere in between there must be a point where you stop ignoring the
return value, and start checking it. Where do you draw this line? It must
depend on whether you will deploy to a low memory or high memory
environment... but is there a good rule?

Yes. You can get away with not checking the value returned by
malloc(N) if N is evenly divisible by all prime numbers (you only
need to test divisors up to sqrt((size_t)-1); any larger primes can
be ignored). For all other values of N, you must check the value
returned by malloc() -- and similarly for calloc() and realloc(),
of course.

 

[Seebs]

Hello friends~~

Think about malloc.

Obviously, for tiny allocations like 20 bytes to strcpy a filename,
there's no point putting in a check on the return value of malloc... if
there is so little memory then stack allocations will also be failing and
your program will be dead.

Not necessarily true -- there is no reason to assume that "stack allocations"
and malloc() are using the same pool of memory. Furthermore, it's quite
possible for malloc to fail, not because 20 bytes aren't available, but
because it can't get enough space for the 20 bytes plus overhead.

So somewhere in between there must be a point where you stop ignoring the
return value, and start checking it. Where do you draw this line? It must
depend on whether you will deploy to a low memory or high memory
environment... but is there a good rule?

There is -- ALWAYS check.

-s

 

[Richard Bos]

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

Wrong.

Richard

 

[Gene]

Hello friends~~

Think about malloc.

Obviously, for tiny allocations like 20 bytes to strcpy a filename,
there's no point putting in a check on the return value of malloc... if
there is so little memory then stack allocations will also be failing and
your program will be dead.

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

So somewhere in between there must be a point where you stop ignoring the
return value, and start checking it. Where do you draw this line? It must
depend on whether you will deploy to a low memory or high memory
environment... but is there a good rule?

Regards~~

As has been said, in production code, you always check. However this
does not mean you have to code a specific check for each allocation.

The usual is to define a wrapper around malloc() that checks for null
returns and deals with them through a callback mechanism where
presumably resources are freed so the allocation can succeed and/or a
longjmp() that terminates the application gracefully.

One thing that hasn't been mentioned about heap storage is
fragmentation. Even if you are allocating 20 bytes, malloc() can
still fail with (theoretically) 19/20 = 95% memory free. The
improbable, the moral is that when heap allocation fails, an app keep
running by compacting the heap. If you aren't checking each
allocation, this doesn't work.

 

[sandeep]

For a proper application, especially to be run by someone else on their
own machine, then you should check allocations of any size (and have the
machinery in place to deal with failures sensibly).

For anything else, where you don't expect a failure, or it is not a big
deal, then use a wrapper function around malloc(). That wrapper will
itself check, and abort in the unlikely event of a memory failure. But
it means you don't have to bother with it in your main code.

This is a good idea. I have just made a clever macro to do this - not as
easy as it seems due to void use problems and need for a temporary.

static void* __p;
#define safeMalloc(x) ((__p=malloc(x))?__p:\
(exit(printf("unspecified error")),(void*)0))

 

[Seebs]

This is a good idea. I have just made a clever macro to do this - not as
easy as it seems due to void use problems and need for a temporary.

static void* __p;
#define safeMalloc(x) ((__p=malloc(x))?__p:\
(exit(printf("unspecified error")),(void*)0))

Write a function, not a macro, it'll be easier to make effective use of.

Also.

1. Use fprintf(stderr,...) for error messages.
2. Terminate error messages with newlines.
3. Why the *HELL* would you use "unspecified error" as the error message
when you have ABSOLUTE CERTAINTY of what the error is? Why not:
fprintf(stderr, "Allocation of %ld bytes failed.\n", (unsigned long) x);

The first two are comprehensible mistakes. The third isn't. Under what
POSSIBLE circumstances could you think that "unspecified error" is a better
diagnostic than something that in some way indicates that a memory allocation
failed?

I really don't understand this one. Please try to explain your reasoning,
because I regularly encounter software that fails with messages like these,
and I've always assumed it was something people did out of active malice --
they hate their users and want the users to suffer. If there is any other
possible reason to, given total certainty of what the problem is, refuse
to hint at it, I do not know what it is.

-s

 

[Eric Sosman]

Yes. ALWAYS check for error returns.

What you do with an error depends on your application. If it's an
unpublished personal utility an error exit might be sufficient.
Otherwise, more sophisticated error diagnostic messages or recovery
might be needed for larger or published (production) programs.

I often use this pair of functions in toy programs:

void crash(const char *message) {
if (message != NULL)
perror (message);
exit (EXIT_FAILURE);
}

void *getmem(size_t bytes) {
void *new = malloc(bytes);
if (new == NULL && bytes > 0)
crash ("malloc");
return new;
}

Note the limitation to "toy programs." In larger programs, low-
level functions like memory allocators lack information about the
context in which a failure occurs, and so can't make informed
decisions about what should be done. This getmem() is far too
ill-mannered and abrupt for "serious" use, because it can't tell
the difference between a recoverable failure ("Not enough memory;
close some windows and try again"), and an irrecoverable failure
("Not enough memory; shutting down"). Even in the latter case, the
program may want to do a few last-gasp things like saving the user's
work to disk before going away to push up daisies; a getmem() that
just called exit() and ripped the rug from underneath the rest of
the program would be unwelcome indeed.

... and a program that simply didn't check at all but died
with a SIGSEGV or equivalent would be even worse. Even my high-
handed getmem() allows for the possibility of atexit() routines,
while dereferencing NULL does not.

 

[Ian Collins]

This is a good idea. I have just made a clever macro to do this - not as
easy as it seems due to void use problems and need for a temporary.

static void* __p;
#define safeMalloc(x) ((__p=malloc(x))?__p:\
(exit(printf("unspecified error")),(void*)0))

Why mess about with a macro when a function would do?

 

[sandeep]

Write a function, not a macro, it'll be easier to make effective use of.
??
How?

Also.

1. Use fprintf(stderr,...) for error messages. 2. Terminate error
messages with newlines. 3. Why the *HELL* would you use "unspecified
error" as the error message when you have ABSOLUTE CERTAINTY of what the
error is? Why not:
fprintf(stderr, "Allocation of %ld bytes failed.\n", (unsigned long)
x);

The first two are comprehensible mistakes. The third isn't. Under what
POSSIBLE circumstances could you think that "unspecified error" is a
better diagnostic than something that in some way indicates that a
memory allocation failed?

I really don't understand this one. Please try to explain your
reasoning, because I regularly encounter software that fails with
messages like these, and I've always assumed it was something people did
out of active malice -- they hate their users and want the users to
suffer. If there is any other possible reason to, given total certainty
of what the problem is, refuse to hint at it, I do not know what it is.

Many users will only be confused by technical error messages about memory
allocation etc. It's best not to get into unwanted details - the user
doesn't know about how my program allocates memory, it just needs to know
there was an error that needs a restart. I think in books they call it
leaking abstractions.

 

[sandeep]

Why mess about with a macro when a function would do?

Obviously for efficiency! malloc may be called many times in the course
of a program.

 

[Seebs]

??
How?

This question is too incoherent to answer.

What part of "a function" do you have trouble with? You know how to write
functions, right? You know how to call them, right?

Try adding some verbs. Questions like "how do I declare a function" or "how
do I use a function" might begin to be answerable. An explanation of what
you're having trouble with, specifically, would be even better.

Many users will only be confused by technical error messages about memory
allocation etc. It's best not to get into unwanted details - the user
doesn't know about how my program allocates memory, it just needs to know
there was an error that needs a restart. I think in books they call it
leaking abstractions.

Wrong.

Users who are "confused" by an error message can accept that they got "an
error". MANY users, however, know enough to recognize that "out of memory"
is different from "file not found".

Stop trying to outsmart the user. Tell the truth, always. It's fine to
stop short of a register and stack dump, but at least tell people honestly
and accurately what happened.

Where did you get this bullshit? The above paragraph is by far the stupidest
thing I've ever seen you write. It's not just a little wrong; it's not just a
little stupid; it's not just a little callous or unthinking. It's one of
the most thoroughly, insideously, wrong, stupid, and evil things you could
start thinking as a programmer.

Stop. Rethink. THINK AHEAD. For ****'s sake, JUST THINK EVEN A TINY BIT
AT ALL.

1. Users will report the error message to you. You need that error message
to give you the information you need to track down the problem.

2. "Error that needs a restart" is nearly always bullshit. If the program
is running out of memory because you made a mistake causing it to try to
allocate 4GB of memory on a 2GB machine, "restart" will not fix it. Nothing
will fix it until the user finds out what's wrong and submits a bug report
allowing the developer to fix it.

3. "Error that needs a restart" is at best a surrender to inability to fix
bugs. If restarting makes the error "go away", you have a serious bug that
you ought to fix.

4. The chances are very good that many of the prospective users of any
program will, in fact, be able to program at least a little, or will have
basic computer literacy. From what I've seen of your posts in this newsgroup,
I'd guess a large proportion of the users of any software you write will,
in fact, know more about computers than you do.

5. Trying to avoid "confusing" people is power-mad idiocy. Your job here
is not to imagine yourself some kind of arbiter-of-technology, preserving the
poor helpless idiots from the dangers of actual information. Your job is
to make a program which works as well as possible, and that includes CLEAR
statements of what failed.

6. You can never make a message so clear that every concievable user will
understand it. However, a user who won't understand a simple message won't
understand an imprecise or flatly false one, either. There does not exist
a user who will have a clear idea of what went wrong and be able to react
accordingly when confronted with "unspecified error", but who will be utterly
paralyzed like a deer in headlights when confronted with "memory allocation
failed". As a result, even if we restrict our study to the set of users
who simply have no clue what those words mean, you STILL gain no benefit,
at all, from the bad message. But in the real world, you hurt many of your
users by denying them the information that would allow them to address
the issue (say, by closing other applications so that more memory becomes
available).

7. Again, don't lie to people. If you know it's a memory allocation failure,
say so.

Let me put it this way: If I saw that error message in a code sample
submitted by an applicant, that would be the end of the interview process.
I would never hire someone who wrote that. I would not want to take on
the full-time job of correcting badly-written error messages from a developer
who holds users in such contempt.

And make no mistake. By asserting that users will "just be confused", you
are indeed showing contempt for them. You're ignoring the fact that users
are, by and large, humans. If they can read the text "unspecified error",
they are well along the way to being smart enough to understand a lot more
than just that, and they probably are.

Furthermore, "many" is not "all". If writing an informative error message
required a week's effort, I could understand choosing not to. But in this
case, writing an informative message requires NO EFFORT AT ALL. You don't
have to do anything better to provide an informative message than to provide
an uninformative one. Sure, it could take more effort to produce a really
good message, such as "Allocation of 256 bytes failed at list.c, line 623".
But simply writing "memory allocation failed" is not noticably harder than
writing "unspecified error". And yet, for every one of your users who *isn't*
a terrified, panicked, idiot, you've just condemned them to a useless error
message because you *think* they *might* be too stupid or ignorant to own
a computer.

In short, this is a catastrophically bad design approach. Abandon it. Reject
it. Anything you think you know which caused you to adopt this is probably
also utterly wrong, and dangerously so, and until you root all the madness
out and start afresh, your code will be dangerous, untrustworthy, and based
on a bad attitude.

Respect the user. Worst case, the user doesn't get it, but they're no worse
off. You could learn a lot from looking at, say, Blizzard Entertainment.
Their userbase consists largely of marginally-literate teenagers. And yet,
when they encounter errors, they give clear, detailed, error messages which
can be forwarded to their support team to allow the developers to fix
problems.

I have spent an occasional idle afternoon reading their support forums. I
have never, in all that time, seen a user say "help, World of Warcraft
crashed, and it used a word I don't know, is it okay for me to restart
it?"

-s

 

[Seebs]

Obviously for efficiency! malloc may be called many times in the course
of a program.

Please stop trying to outsmart the compiler.

The "cost" of using a function instead of a macro is likely to be so small
that you can't even measure it. If there is even a cost at all, which there
may not be.

Premature optimization is the root of a great deal of evil. Do not try
to optimize something like this until you have PROVEN that it is actually
a significant bottleneck. In general, it won't be. In fact, I do not
think I have ever, in something like twenty years of active use of C, seen
a single program in which such a thing would have been a measurable component
(say, over one hundredth of one percent) of runtime.

-s

 

[Ian Collins]

Obviously for efficiency! malloc may be called many times in the course
of a program.

Obviously any compiler worth using will inline such a trivial function away.

 

[Keith Thompson]

This is a good idea. I have just made a clever macro to do this - not as
easy as it seems due to void use problems and need for a temporary.

static void* __p;
#define safeMalloc(x) ((__p=malloc(x))?__p:\
(exit(printf("unspecified error")),(void*)0))

Names starting with two underscores, or with an underscore and
a capital letter, are reserved to the implementation for all
purposes. Other names starting with an underscore are reserved in
more limited ways. Basically you should *never* define your own
identifier starting with an underscore (unless you're writing a C
implementation yourself).

Seebs already covered the "unspecified error" message, and I agree
with him. Just say "memory allocation failure\n" (note the trailing
newline!) or something similar. Any naive users who don't understand
what that means will just read it as "unspecified error" anyway,
and users who do understand it just might be able to do something
about it (such as shutting down other programs and trying again,
or running the program with a smaller input file, or ...).

You might also consider adding a parameter to allow the caller to
pass in a string saying what was going on when the allocation failed.

printf() returns the number of characters printed. Passing this
value to exit() makes no sense at all. What is an exit status of
17 supposed to mean?

void *safeMalloc(size_t size)
{
void *const result = malloc(size);
if (result == NULL) {
fputs("Memory allocation failure\n", stderr);
exit(EXIT_FAILURE);
}
else {
return result;
}
}

If you're concerned about performance, just declare it inline.

 

[Keith Thompson]

Stop trying to outsmart the user. Tell the truth, always. It's fine to
stop short of a register and stack dump, but at least tell people honestly
and accurately what happened.

Where did you get this bullshit? The above paragraph is by far the stupidest
thing I've ever seen you write. It's not just a little wrong; it's not just a
little stupid; it's not just a little callous or unthinking. It's one of
the most thoroughly, insideously, wrong, stupid, and evil things you could
start thinking as a programmer.

Stop. Rethink. THINK AHEAD. For ****'s sake, JUST THINK EVEN A TINY BIT
AT ALL.

Um, Seebs, maybe you should try decaf?

 

[Keith Thompson]

Many users will only be confused by technical error messages about memory
allocation etc. [...]
I think in books they call it leaking abstractions.

Stop reading those books immediately.

At least until you can understand what they're saying. I rather
doubt that books discussing "leaking abstractions" (a useful concept
and something to avoid) would recommend an "unspecified error"
message over "memory allocation failed".