Velocity Reviews - Computer Hardware Reviews

Velocity Reviews > Newsgroups > Programming > C Programming > Cleanup patterns

Reply
Thread Tools

Cleanup patterns

 
 
MQ
Guest
Posts: n/a
 
      11-29-2006
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_function()
{
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;
}

 
Reply With Quote
 
 
 
 
Tom St Denis
Guest
Posts: n/a
 
      11-29-2006

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_function()
> {
> 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

 
Reply With Quote
 
 
 
 
Eric Sosman
Guest
Posts: n/a
 
      11-29-2006
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_function()
> {
> 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
http://www.velocityreviews.com/forums/(E-Mail Removed)lid

 
Reply With Quote
 
MQ
Guest
Posts: n/a
 
      11-29-2006

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.

 
Reply With Quote
 
CBFalconer
Guest
Posts: n/a
 
      11-29-2006
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_function()
> {
> 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>


 
Reply With Quote
 
Eric Sosman
Guest
Posts: n/a
 
      11-29-2006
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
(E-Mail Removed)lid

 
Reply With Quote
 
MQ
Guest
Posts: n/a
 
      11-29-2006

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

 
Reply With Quote
 
Richard Heathfield
Guest
Posts: n/a
 
      11-29-2006
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.
 
Reply With Quote
 
Chris Dollin
Guest
Posts: n/a
 
      11-29-2006
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_function()
> {
> 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/

 
Reply With Quote
 
Eric Sosman
Guest
Posts: n/a
 
      11-29-2006
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
>>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_function()
>>{
>> 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
(E-Mail Removed)lid
 
Reply With Quote
 
 
 
Reply

Thread Tools

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

BB code is On
Smilies are On
[IMG] code is On
HTML code is Off
Trackbacks are On
Pingbacks are On
Refbacks are Off


Similar Threads
Thread Thread Starter Forum Replies Last Post
Registry Cleanup - Hardware Drivers =?Utf-8?B?TGFycnktTGFzZXI=?= Wireless Networking 1 12-16-2005 08:57 PM
Symantec Express Cleanup or alternative Ron P Firefox 2 04-24-2005 10:39 AM
Downloads "cleanup" in Firefox Jim Firefox 2 02-15-2005 04:02 PM
where to find good patterns and sources of patterns (was Re: singletons) crichmon C++ 4 07-07-2004 10:02 PM
Cleanup Newsgroups miskairal Firefox 5 05-04-2004 02:49 AM



Advertisments