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;
}

[Tom St Denis]

MQ wrote:

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

Don't use macros with jumps and what not.

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

b and c are not pointers [or at least not at the same level as a]

errno = 0;

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

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

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

I'd do simply

a = malloc(sizeof(* a));
if (a == NULL) { errno = outofmem; goto ERR_A; }
b = ...
if (b == NULL) { errno = outofmem; goto ERR_B; }
....

Then at the end have

errno = 0;

ERR_C:
free(b);
ERR_B:
free(a);
ERR_A:
return errno;
}

Makes the code easy to follow and doesn't involve nasty if's all over
the place (e.g. if you avoided goto all together..)

Tom

[Eric Sosman]

MQ wrote:

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.

In C, it's typical to use nested if's, and/or carefully
initialized variables, and/or decomposition into multiple
functions, and/or goto.

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.

Your method is Bad(tm). For starters, it won't compile.
That could be fixed by changing the macro definition, but then
you'd be invoking undefined behavior if either of the first two
malloc() calls failed. Also, Standard C doesn't define ENOMEM,
and `errno!=0' is not a valid test for failure.

All of these problems are easily fixed or worked around, but
consider: You've advanced CLEANUP as, essentially, a way to help
write better programs. Since it demonstrably has *not* helped
you to write a better program, perhaps it's time to question its
effectiveness, no?

#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;
}

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

[MQ]

Eric Sosman wrote:

MQ wrote:

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.

In C, it's typical to use nested if's, and/or carefully
initialized variables, and/or decomposition into multiple
functions, and/or goto.

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.

Your method is Bad(tm). For starters, it won't compile.
That could be fixed by changing the macro definition, but then
you'd be invoking undefined behavior if either of the first two
malloc() calls failed. Also, Standard C doesn't define ENOMEM,
and `errno!=0' is not a valid test for failure.

Hi Eric

I did throw this together rather quickly and no doubt there will be a
number of errors, but after posting I fixed these problems and it does
compile fine. Ignoring this, I just want to know if the general
structure of the code is good practise. You should be able to
ascertain what I am trying to achieve...
MQ.

[Eric Sosman]

MQ wrote:

Eric Sosman wrote:

>>MQ wrote:

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

In C, it's typical to use nested if's, and/or carefully
initialized variables, and/or decomposition into multiple
functions, and/or goto.

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

Your method is Bad(tm). For starters, it won't compile.
That could be fixed by changing the macro definition, but then
you'd be invoking undefined behavior if either of the first two
malloc() calls failed. Also, Standard C doesn't define ENOMEM,
and `errno!=0' is not a valid test for failure.

Hi Eric

I did throw this together rather quickly and no doubt there will be a
number of errors, but after posting I fixed these problems and it does
compile fine. Ignoring this, I just want to know if the general
structure of the code is good practise. You should be able to
ascertain what I am trying to achieve...

It appears you are trying to achieve what I described as
typical: "nested if's, and/or carefully initialized variables,
and/or decomposition into multiple functions, and/or goto."
My original question remains: Given that the attempt led you
into four different errors (and perhaps more in the "fixed"
version we haven't seen yet), do you still think you're going
about things in the best way possible? Note that "it does
compile fine" and "it is correct" are not the same thing!

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

[CBFalconer]

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

I would implement that as:

int example(void)
{
SOMETYPE *a, *b, *c;

errno = 0;
a = b = c = NULL;
if (!(a = malloc(sizeof *a))) errno = ENOMEM;
else if (!(b = malloc(sizeof *b))) errno = ENOMEM;
else if (!(c = malloc(sizeof *c))) errno = ENOMEM
else {
/* dosomething here */
}
free(a); free(b); free(c);
return -(0 != errno);
}

Notice the absence of obfuscating macros. Also the routine no
longer frees undefined pointers or non-pointers. No goto needed,
although I do not hesitate to use them when appropriate. I would
normally choose different return values.

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

[MQ]

It appears you are trying to achieve what I described as
typical: "nested if's, and/or carefully initialized variables,
and/or decomposition into multiple functions, and/or goto."
My original question remains: Given that the attempt led you
into four different errors (and perhaps more in the "fixed"
version we haven't seen yet), do you still think you're going
about things in the best way possible?

Umm, well, that was my question originally. I am looking for feedback.
And the question is more generic than the example above. I want to
know *generically* what are the best methods for implementing cleanup
in C functions. I can say for sure that *something* needs to be done,
as a function that holds potentially numerous resources cannot clean up
at the point of error, otherwise most of the code will be cleanup
stuff, and obscure the real task that the code is doing.

MQ.

[Richard Heathfield]

MQ said:

Hi all

I am just wondering how most people implement cleanup in C functions.

I can't speak for most people, but I do it like this:

Attempt to acquire resource
If attempt succeeded
Use resource
Release resource
Else
Handle error
Endif

This code structure is recursively extensible, and functions are a fabulous
mechanism for keeping the indent level down.

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

[Chris Dollin]

MQ wrote:

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;
}

This one I'd write (fixing a couple of things along the way) as:

int exampleFunction ()
{
SomeType *a = malloc( sizeof (*a) );
SomeType *b = malloc( sizeof (*b) );
SomeType *c = malloc( sizeof (*c) );
int status = a && b && c ? 0 : -1;
if (status == 0)
{
/* do what must be done */
}
free( a ), free( b ), free( c );
return status;
}

I don't think your example is complex enough to demonstrate whatever
value your macro might have.

--
Chris "Magenta - the best colour of sound" Dollin
"No-one here is exactly what he appears." G'kar, /Babylon 5/

[Eric Sosman]

CBFalconer wrote:

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) . C++ has exceptions, the only way I can see to do
this neatly in C is to use goto statements. Is my method of
implementin g 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;
}

I would implement that as:

int example(void)
{
SOMETYPE *a, *b, *c;

errno = 0;
a = b = c = NULL;
if (!(a = malloc(sizeof *a))) errno = ENOMEM;
else if (!(b = malloc(sizeof *b))) errno = ENOMEM;
else if (!(c = malloc(sizeof *c))) errno = ENOMEM

Missing a semicolon here, right after the possibly
undefined identifier ENOMEM.

else {
/* dosomething here */
}
free(a); free(b); free(c);
return -(0 != errno);

Note that free() is permitted to set errno to a non-zero
value. So might /* dosomething here */ if it were not a
comment.

errno is not a good holder for program state, because
it's "transient: " most library functions can change it at
will, whether or not any error has occurred. Park a value
in errno, and it may be "gone in sixty seconds."

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