I just noticed this... If your use of goto is to return, why don't you just put an return in place of the goto? Then the intention will be much clearer. With goto to exit, you are just camouflaging the real intent. You are adding one more level of "complexity" that is unnecessary. You are still breaking the flow with return, but it is still cleaner than a goto in this case. Besides, I am not so sure it is all that bad to break the flow in this manner. The intent with a return statement is clear: I am done with this function.
Notice the word "cleanup". This is the key point - many people have brought up C error handling, and for good reason: which idiom do you prefer between these three popular approaches? There are more, but these are just examples. The "sequential error handling" model:
int do_work(int stuff)
{
int res1, res2, res3;
res1 = acquire_one_resource(stuff);
if (!res1)
{
return 0; // error!
}
res2 = acquire_another_resource(stuff);
if (!res2)
{
free_resource(res1); // gotta clean this up
return 0;
}
res3 = acquire_more_resources(stuff);
if (!res3)
{
free_resource(res2); // oh boy, here we go
free_resource(res1);
return 0;
}
/* etc, releasing more and more previous resources each time
* so that we never leave any acquired resource behind */
/* do work with resources */
free_resource(res3); // oh, yeah, let's not forget
free_resource(res2); // wait, didn't we already write this earlier?
free_resource(res1); // maybe it should go into a function.. oh, wait, it can't, can it
return 1; // yay
}
The "cascade deallocation" model:
int do_work(int stuff)
{
int res1, res2, res3;
int success = 0;
res1 = acquire_one_resource(stuff);
if (res1)
{
res2 = acquire_another_resource(stuff);
if (res2)
{
res3 = acquire_more_resources(stuff);
if (res3)
{
/* etc. until you finally have everything */
/* do some stuff with resources */
success = 1;
free_resource(res3); // here we go
}
free_resource(res2);
}
free_resource(res1);
}
return success;
}
Or the honest-to-god goto model:
int do_work(int stuff)
{
int res1 = 0, res2 = 0, res3 = 0;
int success = 0;
res1 = acquire_one_resource(stuff);
if (!res1) goto cleanup;
res2 = acquire_another_resource(stuff);
if (!res2) goto cleanup;
res3 = acquire_more_resources(stuff);
if (!res3) goto cleanup;
/* etc. */
success = 1;
cleanup:
/* here the only requirement is that the cleanup process
* ignores unacquired resources, perhaps with some minor
* logic if you have pointers to resources (unusual) */
free_resource(res3);
free_resource(res2); // would you look at that, all the cleanup code
free_resource(res1); // in one single spot, no code duplication
return success;
}
(there's also the variant with multiple labels that jump at the appropriate position in the cleanup routine, which relaxes the requirement of having to ignore resources which were never acquired, but that I find a bit too verbose for my taste personally - if you are on a relatively flat memory model, which you should be in C, cleanup itself will be easy, the harder part is determining what to clean up at any given time)
One of the snippets above scales. The other two don't. I think it's clear which is which.
In C++ and other languages you have stuff to help you. In C++ that's the RAII memory ownership model, in C# or Java that's the garbage collector and maybe some other automatic acquire/release mechanism (like scoped using declarations in C#), which basically do the same thing but in a mechanical, behind-the-scenes way, which is - of course - better. In C, you have nothing but you and your pointers, and for most nontrivial programs error handling is going to make up a large fraction of the code, so it better be readable and maintainable.
Sure, you can delegate the resource acquisition code to some other part of your program, or stretch your code out over an interminable amount of two-line helper functions in the name of structured programming, but it doesn't matter how much you move it around, eventually you're going to have to deal with the necessity of having multiple resources around at the same time and to be prepared for the possibility of only acquiring part of those resources and being required to execute a correct cleanup sequence every time.
And if you are thinking why I didn't just move the cleanup bit into its own function, let me ask you this: why would I create an off-screen helper function with critical cleanup code that intrinsically needs to be kept in sync with the do_work() function, when I can just have it naturally at the bottom of the function all by replacing "cleanup(); return 0;" with "goto cleanup;". What concrete, practical advantage is there? None. It's a waste of space and programmer energy, and a prime example of cargo cult programming.
If you want to think about this in a real technical way, you should be able to draw a flow chart of your function. This should be done in advance, but who does that? The rules of a flow chart is: that you are having only one entry point, and only one exit point, and that absolutely no lines are crossing each other. No, you can't cheat and draw a flow chart that branches to the right side, and then to the left side to avoid a crossing on the right side. If you are ever using any of goto, break, continue or early returns, then you are breaking this flow.
That's an arbitrary blanket rule. Early returns are a good thing. Breaks and continues have their places. Goto has its place too when you need some kind of control flow that improves readability (like the error handling example) and no other language construct naturally lends itself to that (admittedly, that is rare even in languages like C, and basically nonexistent for higher level languages). And when I see this kind of rule being repeated blindly all I can think of is the programmer who, told not to use goto, wrapped up his error handling code in a dreadful do{}while(0); loop to obtain the same control flow without writing 'goto' and obfuscating his code in the process. And that is far worse than any amount of gotos (well, almost any amount).
I'm not saying goto is good. Here the use of goto for error handling is justifiable because goto naturally lends itself to a rather elegant, very low boilerplate resource cleanup implementation in C. Clearly jumping all over your code, to labels hundreds of lines away, is nothing short of insane. But simply outright banning goto because there is a paper called 'goto considered harmful' without even considering the possibility that maybe, just maybe, goto could be used in a controlled manner (it is a tool, and just like any other tool it can be abused; break and continue are also tools which can be abused) simply shows a lack of intellectual depth in my opinion.
) that I didn't say. I said the comparison between break/continue and goto was unfair because goto is worse than break & continue. I didn't say break/continue were good practice. I try to avoid them too (however for goto, I just never use it).
