Cleanup patterns

[MQ]

Hi all

I am just wondering how most people implement cleanup in C functions.
In particular, if the function opens a number of resources, these need
to be released properly should an error occur at any point in the
function (as well as at the end if successful). C++ has exceptions,
the only way I can see to do this neatly in C is to use goto
statements. Is my method of implementing cleanup good, or are their
better ways. Here is an example, which uses the global errno to store
the error.

#define CLEANUP(err) ({errno = (err); goto cleanup})

int example_functio n()
{
SOMETYPE * a,b,c;
errno = 0;

if(!(a = malloc(sizeof(a ))))
CLEANUP(ENOMEM) ;

if(!(b = malloc(sizeof(b ))))
CLEANUP(ENOMEM) ;

if(!(c = malloc(sizeof(c ))))
CLEANUP(ENOMEM) ;

/* do something here */

cleanup:
if(a)
free(a);
if(b);
free(b);
if(c)
free(c);
if(errno)
return -1;
return 0;
}

[Keith Thompson]

rp*****@yahoo.c om (Roland Pibinger) writes:

On 28 Nov 2006 18:32:42 -0800, "MQ" wrote:

>>I am just wondering how most people implement cleanup in C functions.
In particular, if the function opens a number of resources, these need
to be released properly should an error occur at any point in the
function (as well as at the end if successful).

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/dviwa...2xx/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_FAILU RE);
}
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.

--
Keith Thompson (The_Other_Keit h) ks***@mib.org <http://www.ghoti.net/~kst>
San Diego Supercomputer Center <* <http://users.sdsc.edu/~kst>
We must do something. This is something. Therefore, we must do this.

[CBFalconer]

Roland Pibinger wrote:

On Sat, 09 Dec 2006 12:47:38 -0500, CBFalconer wrote:

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

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

>

>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?

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

--
Chuck F (cbfalconer at maineline dot net)
Available for consulting/temporary embedded and systems.
<http://cbfalconer.home .att.net>

[websnarf]

Roland Pibinger wrote:

On 28 Nov 2006 18:32:42 -0800, "MQ" wrote:

I am just wondering how most people implement cleanup in C functions.
In particular, if the function opens a number of resources, these need
to be released properly should an error occur at any point in the
function (as well as at the end if successful).

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/dviwa...2xx/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) betterMallocInt ernal (sz, __FILE__, __LINE__);
extern void * betterMallocInt ernal (size_t sz, const char * file, int
line);

/* The possible kinds of failures we detect */
enum mallocErrs { MALLOC_ERR_ZERO = 0, MALLOC_ERR_NO_M EM = 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 * betterMallocInt ernal (size_t sz, const char * file, int line) {
void * ptr;

if (!sz) {
mallocLogFail (MALLOC_ERR_ZER O, 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.

--
Paul Hsieh
http://www.pobox.com/~qed/
http://bstring.sf.net/

[Richard Heathfield]

Roland Pibinger said:

On Sat, 09 Dec 2006 17:50:09 +0000, Richard Heathfield wrote:

>>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:

<3D********** *****@eton.powe rnet.co.uk>

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.

--
Richard Heathfield
"Usenet is a strange place" - dmr 29/7/1999
http://www.cpax.org.uk
email: rjh at the above domain, - www.

[Roland Pibinger]

On 9 Dec 2006 08:32:27 -0800, "santosh" wrote:

>Roland Pibinger wrote:

>http://www.tug.org/tex-archive/dviwa...2xx/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.

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]

Roland Pibinger wrote:

On Sat, 09 Dec 2006 12:47:38 -0500, CBFalconer wrote:

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

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

--
Eric Sosman
es*****@acm-dot-org.invalid

[Keith Thompson]

Eric Sosman <es*****@acm-dot-org.invalidwrit es:

Roland Pibinger wrote:

>On Sat, 09 Dec 2006 12:47:38 -0500, CBFalconer wrote:

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

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.

--
Keith Thompson (The_Other_Keit h) ks***@mib.org <http://www.ghoti.net/~kst>
San Diego Supercomputer Center <* <http://users.sdsc.edu/~kst>
We must do something. This is something. Therefore, we must do this.

[Eric Sosman]

Keith Thompson wrote:

Eric Sosman <es*****@acm-dot-org.invalidwrit es:

>Roland Pibinger wrote:

>>On Sat, 09 Dec 2006 12:47:38 -0500, CBFalconer wrote:
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.

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.

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.

--
Eric Sosman
es*****@acm-dot-org.invalid

[Roland Pibinger]

On Sun, 10 Dec 2006 09:11:17 -0500, Eric Sosman wrote:

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 <es*****@acm-dot-org.invalidwrit es:

<snip>

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

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!

--
Richard Heathfield
"Usenet is a strange place" - dmr 29/7/1999
http://www.cpax.org.uk
email: rjh at the above domain, - www.