Cleanup patterns

[santosh]

Not all resources are equal and need equal treatment. It hardly makes
sense to check the return value of malloc. Just use a xmalloc function
which can be found in many variants on the internet (e.g.
http://www.tug.org/tex-archive/dviware/dvi2xx/xmalloc.c).

Well, it's so simple that most programmers will write their own
versions of it, if need be. The code seems to use the old style
function definition and uses unsigned instead of size_t, which isn't
the way it's done now. And calling exit() with a random value is not
portable. It's better to use EXIT_FAILURE.

In short that code is _old_. Better to rewrite it on case-by-case
basis, (particularly since it's trivial.)

 

[Richard Heathfield]

Roland Pibinger said:

It hardly makes sense to check the return value of malloc.

It certainly doesn't make sense not to!

 

[Ben Pfaff]

Not all resources are equal and need equal treatment. It hardly makes
sense to check the return value of malloc. Just use a xmalloc function
which can be found in many variants on the internet (e.g.
http://www.tug.org/tex-archive/dviware/dvi2xx/xmalloc.c).

Here's that implementation:

void *
xmalloc (size)
unsigned size;
{
void *new_mem = (void *) malloc (size);

if (new_mem == NULL)
{
fprintf (stderr, "fatal: memory exhausted (xmalloc of %u bytes).\n",
size);
/* 1 means success on VMS, so pick a random number (ASCII `K'). */
exit (75);
}

return new_mem;
}

This is buggy: calling xmalloc(0) on some systems will always
cause the program to exit with a claim that memory is exhausted,
which is not (necessarily) true.

Also, EXIT_FAILURE is preferred to "exit (75)", there is no need
to cast the return value of malloc(), and size_t would be
preferred to "unsigned" to measure the size of an object.
(However, it is likely that this is pre-Standard code.)

 

[matevzb]

Not all resources are equal and need equal treatment. It hardly makes
sense to check the return value of malloc. Just use a xmalloc function
which can be found in many variants on the internet (e.g.
http://www.tug.org/tex-archive/dviware/dvi2xx/xmalloc.c).

Best wishes,
Roland Pibinger

Hmm... this may be okay for smaller programs, but would you really want
to just exit() in larger applications? And I think malloc() can fail
even though there still is (some) memory available, it might just be
too fragmented to allocate the requested size. There are also two other
"problems" with xmalloc(). First, it declares malloc() as being
extern char *malloc ();
instead of
void *malloc (size_t size);
Second, xmalloc() in this example uses unsigned for the size (instead
of size_t).

 

[Roland Pibinger]

Roland Pibinger said:


It certainly doesn't make sense not to!

And what is your program supposed to do when memory is exhausted?

 

[Guillaume]

Not all resources are equal and need equal treatment. It hardly makes
sense to check the return value of malloc. Just use a xmalloc function
which can be found in many variants on the internet (e.g.
http://www.tug.org/tex-archive/dviware/dvi2xx/xmalloc.c).

Good one.
I hope you don't write safety-critical software.

 

[CBFalconer]

And what is your program supposed to do when memory is exhausted?

For example, it can save the data it has collected to a file, send
a message, and exit gracefully. Consider a big sort where the
input mechanism collects records in a linked list until memory is
exhauseted, mergesorts the list, dumps it to a temporary file,
discards the list, and repeats, dumping to the next temporary
file. When the input is exhausted it mergest the set of
temporaries into an output file.

 

[Richard Heathfield]

Roland Pibinger said:

And what is your program supposed to do when memory is exhausted?

This is documented in the literature, and has been discussed on this
newsgroup a number of times before. See, for example:

<[email protected]>

or

<[email protected]>

where I bullet-point a few ideas.

 

[Roland Pibinger]

Roland Pibinger said:

This is documented in the literature, and has been discussed on this
newsgroup a number of times before. See, for example:

<[email protected]>

Those are mostly hints how to reduce the dynamic memory consumption of
a program but don't answer the question. OOM usually means that you
must terminate the application. The simplest way is a xmalloc-like
function. If enough context is available some cleanup can be done
before.

Best regards,
Roland Pibinger

 

[Roland Pibinger]

For example, it can save the data it has collected to a file, send
a message, and exit gracefully.

All those functions must guarantee that neither they themself nor any
called function calls *alloc.

Consider a big sort where the
input mechanism collects records in a linked list until memory is
exhauseted, mergesorts the list, dumps it to a temporary file,
discards the list, and repeats, dumping to the next temporary
file. When the input is exhausted it mergest the set of
temporaries into an output file.

Isn't there an external mergesort?

Best regards,
Roland Pibinger

 

[CBFalconer]

All those functions must guarantee that neither they themself nor
any called function calls *alloc.

Nonsense. At most it must open the putative output file ahead of
time.

Isn't there an external mergesort?

That's what I am building above. These things don't grow on trees.

 

[Keith Thompson]

Not all resources are equal and need equal treatment. It hardly makes
sense to check the return value of malloc. Just use a xmalloc function
which can be found in many variants on the internet (e.g.
http://www.tug.org/tex-archive/dviware/dvi2xx/xmalloc.c).

I just took a look at xmalloc.c. Ugh.

I suppose it was written to be compatible with pre-ANSI compilers,
something that almost certainly is no longer necessary.

On failure, it prints a message to stderr and terminates the program:

/* 1 means success on VMS, so pick a random number (ASCII `K'). */
exit (75);

Apparently the author wasn't aware that any odd value means success on
VMS.

But those are just details, easily fixed. Here's a more modern
version (it's covered by the GPL, I suppose):

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

void *xmalloc(size_t size)
{
void *result = malloc(size);
if (result == NULL) {
fprintf(stderr,
"fatal: memory exhausted (xmalloc of %lu bytes).\n",
(unsigned long)size);
exit(EXIT_FAILURE);
}
return result;
}

(A value other than EXIT_FAILURE could be sensible if the code isn't
intended to be 100% portable.)

The problem with this is that it doesn't give the program using it an
opportunity to handle the error itself. If that's what you want -- if
you want to terminate the program immediately if there's an allocation
failure -- then it's not a bad solution. If you want to be able to
recover from an allocation failure, though, then this obviously isn't
what you want to use; instead, you can just call malloc() directly and
add whatever error-handling code you like. And yes, there are cases
where you want to do more than terminate the program on an allocation
failure.

You wrote:

It hardly makes sense to check the return value of malloc.

but that's *exactly* what xmalloc() does.

 

[websnarf]

Not all resources are equal and need equal treatment. It hardly makes
sense to check the return value of malloc. Just use a xmalloc function
which can be found in many variants on the internet (e.g.
http://www.tug.org/tex-archive/dviware/dvi2xx/xmalloc.c).

Besides the objections from the other posters here, this code is
seriously inflexible. You *may* want to ignore the error, or you may
wish to deal with it at the callsite, or maybe you just want assistance
with debugging. But probably, you want some combination of those
three. It pays to make your solution more powerful. For example:

====== ymalloc.h ==============================
/* Just use ymalloc in place of malloc */
#define ymalloc(sz) betterMallocInternal (sz, __FILE__, __LINE__);
extern void * betterMallocInternal (size_t sz, const char * file, int
line);

/* The possible kinds of failures we detect */
enum mallocErrs { MALLOC_ERR_ZERO = 0, MALLOC_ERR_NO_MEM = 1 };

/* Settable error handler for ymalloc (might not be thread safe.) */
extern void (* mallocLogFail) (enum mallocErrs errkind, const char *
file, int line, size_t sz);

====== ymalloc.c ==============================
#include <stdlib.h>

static void logFail (enum mallocErrs errkind, const char * file, int
line, size_t sz) {
char * msgs[2] = { "", "Out of memory; " };
fprintf (stderr, "fatal: %smalloc of %lu bytes at %s[%d].\n",
msgs[errkind], (unsigned long) sz, file, line);
exit (EXIT_FAILURE);
}

void (* mallocLogFail) (enum mallocErrs errkind, const char * file, int
line, size_t sz) = logFail;

void * betterMallocInternal (size_t sz, const char * file, int line) {
void * ptr;

if (!sz) {
mallocLogFail (MALLOC_ERR_ZERO, file, line, sz);
return NULL;
}
if (NULL == (ptr = malloc (sz))) {
mallocLogFail (MALLOC_ERR_NO_MEM, file, line, sz);
}
return ptr;
}
===============================================

The point is that you can choose the policy about what to do about the
failing malloc on a scenario by scenario basis. Malloc() is a slow
enough function, that adding in any arbitrary amount of structure,
debugging, analysis etc, is usually well worth it for the relative
cost.

 

[Richard Heathfield]

Roland Pibinger said:

Those are mostly hints how to reduce the dynamic memory consumption of
a program but don't answer the question.

No, they're suggestions on how to cope when memory is running low - e.g.
when malloc returns NULL (which is a pretty big hint that memory is running
low, is it not?).

OOM usually means that you must terminate the application.

No, it means that *you* must terminate the application (what I call the
"Student Solution", because normally it won't actually lose you any marks
in college). I prefer to deal with matters more robustly.

 

[Roland Pibinger]

Well, it's so simple that most programmers will write their own
versions of it, if need be. The code seems to use the old style
function definition and uses unsigned instead of size_t, which isn't
the way it's done now. And calling exit() with a random value is not
portable. It's better to use EXIT_FAILURE.

Agreed, it was the first example I found on the internet. Google
returns 'about 440,000' hits for 'xmalloc'.

In short that code is _old_. Better to rewrite it on case-by-case
basis, (particularly since it's trivial.)

Agreed again.

Best regards,
Roland Pibinger

 

[Eric Sosman]

All those functions must guarantee that neither they themself nor any
called function calls *alloc.

A fairly common strategy is to malloc() a good-sized
chunk of memory "for emergency use only," sometime during
program initialization when memory is probably plentiful.
If malloc() fails later on, you free() the emergency stash
to provide enough memory for the graceful-shutdown code to
subsist on.

A practical difficulty with this strategy is that it's
hard to estimate what "good-sized" ought to be, and that the
required amount tends to change as time passes and features
are added to the program. Somebody adds an innocent-appearing
but memory-hungry callback via atexit(), and all of a sudden
the stash proves inadequate ... The remedy, I think, is to
include a memory exhaustion test in the regression suite; a
"spiked" version of malloc() et al. can be helpful here. (Of
course, the Standard does not allow one to "spike" a Standard
library function, but there are often sub rosa ways to do so.)

 

[Keith Thompson]

A fairly common strategy is to malloc() a good-sized
chunk of memory "for emergency use only," sometime during
program initialization when memory is probably plentiful.
If malloc() fails later on, you free() the emergency stash
to provide enough memory for the graceful-shutdown code to
subsist on.

The cost of this is that you can run out of memory in cases where
there would have been enough if you hadn't allocated the emergency
stash.

In some cases, you might be able to use some non-critical allocation
as the emergency stash, if you happen to have a sizeable chunk of
allocated memory that you know won't be needed during an emergency
cleanup. But even if that's not possible, I think it's worth the
cost.

 

[Eric Sosman]

The cost of this is that you can run out of memory in cases where
there would have been enough if you hadn't allocated the emergency
stash.

True. Sometimes the canary dies even though the air in
the mine is still just barely breathable.

In some cases, you might be able to use some non-critical allocation
as the emergency stash, if you happen to have a sizeable chunk of
allocated memory that you know won't be needed during an emergency
cleanup. But even if that's not possible, I think it's worth the
cost.

True again: If there's something lying around that you know
won't be needed during the graceful shutdown, that something may
double as the emergency stash. This can be a little more complex
to handle, though. Fragmentation may be a problem (if you wind
up free()ing a lot of little pieces instead of a large chunk),
and there's the question of what to do about malloc() failure
before the disposable data structure is fully constructed. No
reason not to try, though.

 

[Roland Pibinger]

A fairly common strategy is to malloc() a good-sized
chunk of memory "for emergency use only," sometime during
program initialization when memory is probably plentiful.
If malloc() fails later on, you free() the emergency stash
to provide enough memory for the graceful-shutdown code to
subsist on.

This strategy mainly postpones OOM. Moreover, you need enough (global)
context for the "graceful-shutdown code" within you alloc function.

Best regards,
Roland Pibinger

 

[Richard Heathfield]

Keith Thompson said:

Eric Sosman <[email protected]> writes:

The cost of this is that you can run out of memory in cases where
there would have been enough if you hadn't allocated the emergency
stash.

Remember that this subthread was prompted by someone who suggested it's a
waste of time to check malloc's return value at all! Such people never ever
run out of memory, no matter how many Gigabytes of emergency reserve they
allocate. Their programs crash randomly sometimes for no apparent reason,
but at least they never run out of memory!