CV
Blog
Photos
book reviews
downloads
work
links
F.A.Q.
interests
travels
trekking
videogames
programming
playing
miniatures
fitness
programming
C, C++
Java
Shell
misc.
guestbook
Considering Goto Harmful, but...
2010-Mar-16 Tuesday 14:17 CET
Since I started programming in C until a few months ago I religiously practiced the rule "Don't use goto" (totaling for about 23 years of abstinence). I remember I was puzzled at first - coming from BASIC programming, I hardly believed you could get along without the infamous instruction.
To change my habits I took this as a challenge, and in a short time I was able to get rid of the evil statement.
In practice I was helped by a bunch of C statement that are basically disguised goto instructions: break, continue and return.
Break and continue allows you to jump out of a loop or at the next iteration; while return is a jump out of the current function.
Single exit point (i.e. just one return per function) is often preached as a Right Thing, but when programming in C, single exit fights madly with error management, forcing you either to deeply nest conditionals or to add boolean variables with the sole purpose of skipping code in case of error.
Amiga was the first computer I programmed in C, it was an advanced machine for those time, but experimental in many ways. For example Amiga operating system provided you with full multitasking capabilities, but the hardware lacked of an MMU therefore no protected memory was in place. This forced the programmer to be very careful about error conditions - one unhandled error and the entire system could be nuked by a single failing program.
That's probably why I have been always attentive to error handling and graceful exit.
It was back then that I start using the idiom:
bool ok1;
bool ok2;
bool ok3;
ok1 = f1();
ok2 = f2();
ok3 = f3();
if( ok1 && ok2 && ok3 )
{
// f1(), f2() and f3() returned ok.
}
if( ok1 ) free1();
if( ok2 ) free2();
if( ok3 ) free3();
This helps to avoid some nesting, but fails in tracking which function succeeded and which didn't. That could be fine in some situation, but not in others. For example if you have to free some resources allocated in f2(), you have to know if f2() succeeded or not.
Conversely, the idiom below:
bool ok1;
bool ok2;
bool ok3;
ok1 = f1();
ok2 = f2();
ok3 = f3();
if( ok1 && ok2 && ok3 )
{
// f1(), f2() and f3() returned ok.
}
if( ok1 ) free1();
if( ok2 ) free2();
if( ok3 ) free3();
Performs proper cleanup, but fails to capture that f2() has to be executed if, and only if, f1() succeeded.
Then I went the C++ way for several years and gained a markedly object oriented approach.
Using C++ you don't have to worry much about these details if you happen to use the RAII idiom. That is, automatic object (i.e. local instances) gets automatically destroyed when the scope is left regardless of the reason that causes the execution to leave the scope.
In other words, if a function fails, be it with an exception or by reporting a specific error and triggering a return, objects that were built are destroyed, leaving the system in a good, non-leaking state.
Fast forward some years I am back to C programming with a heavy legacy of object oriented approach. This means that I try to design modules in an Object Oriented way - modules define classes, each class has one constructor that prepare the instance for usage. Each class also has one destructor (that may be empty, but this is an implementation detail, so if it changes in the future you don't have to change the calling code).
This is the setting were the C error management issue arose again. I want to mimic a C++-like behavior so that when in the constructor there are 3 "sub-objects" to construct I want that proper clean up (i.e. destructor calls) are invoked in case of error.
If you follow a strictly structured approach (without exception support), you get a very convoluted code:
if( f1_ctor() )
{
if( f2_ctor() )
{
if( f3_ctor() )
{
// succesfull
return true;
}
else
{
f2_dtor();
f1_dtor();
}
}
else
{
f1_dtor();
}
}
return false;
The lack of "fall through" semantic forces you to duplicate code and therefore makes the coding and the maintenance more error prone. In fact, suppose you have to add a third call f0_ctor() that must be called before f1_ctor(). Then you have to change nearly everything, indentation included.
Time to reconsider my mind framework. I would need something that selects a portion of "destructor" sequence. Something like a switch with fall through:
progress = 1;
if( f1_ctor() )
{
progress = 2;
if( f2_ctor() )
{
progress = 3;
if( f3_ctor() )
{
progress = 0
}
}
}
switch( progress )
{
case 0:
return true;
case 3:
f2_dtor();
// fall through
case 2:
f1_dtor();
// fall through
case 1:
return false;
}
This can do, it is somewhat error prone when writing and/or changing the code. If you duplicate one of the progress codes you get a wrong cleanup that can go undetected.
Moreover it doesn't seem to add much to the goto-based error management:
if( !f1_ctor() )
{
goto error1;
}
if( !f2_ctor() )
{
goto error2;
}
if( !f3_ctor() )
{
goto error3;
}
return true;
error3:
f2_dtor();
error2:
f1_dtor();
error1:
return false;
This notation is more terse (thus more readable) and appears to be more robust than the previous ones.
So why should I refrain from the goto statement in this case? There isn't any good motivation.
I don't want to provide any sort of "free for all" authorization in the wild usage of goto instruction. On the contrary my claim is that first you have to become of age without using goto (i.e. write programs for at least 18 years), practice Object Oriented Programming, carefully deal with error handling, then if you find yourself in a language lacking of a suitable error management, you may use goto... only if everything else is worse.
RSS Feed for comments to this posttitle
lear -
2010-Mar-16 Tuesday 18:10 CET
I do not see too much differences between first code and latest (the one with goto) and I prefere the first.
If you like to avoid initialize something if not necessary, you can do:
ok1 = f1();
ok2 = ok1 && f2();
ok3 = ok2 && f3();
so you do not call f2() and f3() if f1() fails
I should have written better
max -
2010-Mar-16 Tuesday 23:05 CET
Hi Lear,
it is only partly a matter of taste :-) the main difference between the first and the last snippet is that the first is not able to tell which function failed and therefore it cannot frees the partially allocated functions in case of failure.
Suppose that f1() performs a malloc, while f2() opens a file. If f3() fails, then the operations performed by f1() and f2() have to be rolled back, i.e. the file has to be closed and the memory freed.
Your example is basically the upper half of the second example in my post.
My goal is to replicate the RAII idiom in C. Consider the following C++ construct:
<code>
class X {
T1 a; T2 b; T3 c;
public:
X()
{}
};
</code>
X constructor constructs first a, then b and eventually c. If c fails throws an exception that causes the destruction of b and a allowing the program to release the resources allocated by T1 and T2 constructors.
Considering that in C you don't have exceptions but you have to communicate the failure either by a return code or a global variable, how would you code the X constructor?
I should have written better too
Lear -
2010-Mar-17 Wednesday 13:48 CET
Hi Max
I understand, you wrote:
if( ok1 ) free1();
if( ok2 ) free2();
if( ok3 ) free3();
I do not think it is too bad. Considering that if f1(), f2() and f3() need a freeX(), probably they allocate something (or as you specified, "open" something), and they will return an HANDLE to the resource.
So you do not really needs other boolean, but you can check if the handle is valid (although bool is not bad thing).
More, if your freeX() checks the handle inside (I'm thiking to the C++ "delete"), you do not need to check it outside like in the example.
Anyway, what I was trying to say yesterday (I realize that I didn't say why) is that I prefere your first exampe respect to the one with goto.
I prefere it becase is more readable. If I look the the first code I understand in one second that you are taking resources, using them and freeing them. Looking to the other, I need 3 or 4 seconds to understand that. Only that.
I wrote the lines with "ok &&" because the only PRO I saw for the goto version is avoiding not needed resource allocations. I specified that can be done in the first example keeping the readability.
Hi,
Lear
evil preprocessor
vmiazzo -
2010-Mar-18 Thursday 11:05 CET
If goto is harmful, preprocessor is evil, so... why don't use it?
Just an idea, not tried.
typedef void (*DTOR_PTR)(void);
#define ACQUIRE_INIT() DTOR_PTR releaseProgress = NULL
#define ACQUIRE(_N_,...) DTOR_PTR release_N_ = NULL; if(releaseProgress!=releaseDummy) { releaseProgress = release_N_ = _N_(__VA_ARGS__); }
#define IF_ACQUIRED() if(releaseProgress!=releaseDummy)
#define RELEASE(_N_) release_N_();
void f(void) {
ACQUIRE_INIT();
ACQUIRE(res1,0,"ciao");
ACQUIRE(res2,0.5f);
IF_ACQUIRED() {
//use 1 and 2
}
RELEASE(res2);
RELEASE(res1);
}
DTOR_PTR res1(int a, char* b) {
//...
return dtor1;
}
void dtor1(void) {
//...
}
DTOR_PTR res2(int a, char* b) {
//...
return dtor2;
}
void dtor2(void) {
//...
}
I forgot
vmiazzo -
2010-Mar-18 Thursday 11:08 CET
res1 and res 2 return releaseDummy if something goes wrong.
void releaseDummy(void) {}
evil + harmful
Max -
2010-Mar-19 Friday 14:00 CET
Lear, now I understand your point. I agree. It took me some days to write the article and the focus shifted from plain to something more convoluted, that's why I failed to see your reason. I agree that solution #1 is the least surprising, but the problem is really code duplication - each failing branch of the if/else structure must contain incrementally the same code. Too easy to miss one place to change when you have to modify the code.
Valentino, your idea is really good and pointed me in what I think is a good direction. I am going to write about it in the next post. Thank you.
Write your comment?
