Re: [PATCH 00/11] Atari Ethernet/USB patch series - for upstream and debian-kernel

[Wouter Verhelst <w@uter.be>]

On 29-03-13 23:47, Christian T. Steigies wrote:
> On Fri, Mar 29, 2013 at 09:38:31PM +0000, Thorsten Glaser wrote:
>>
>> Avoiding gotos for the purpose of avoiding gotos is absolutely
>> silly (as the compiler translates them to jumps, which are the
>> same in asm) and positively harmful (for the legibility of the
>> code; I wish I had goto in mksh???).
> 
> From my Commodore Basic days I still know gotos and spaghetti. I know it
> will be translated to jumps, but I thought we are programming in C and not
> asm, because the readability is better. But maybe I just read too much about
> python recently ;-)

Goto in BASIC is not the same thing as goto in C. In BASIC, a goto
statement can go all over the place. You see a goto, you don't know
where you'll end up. Yes, that is spaghetti code.

In C, a goto can only jump within the same function. It's possible to
overuse it, but error unwinding is one of the few cases where it's
sensible and good to use it. Consider:

int handle = initialize_handle();
if(connect_handle(handle)!=CONNECTION_OK) {
	unintialize_handle(handle);
	return ECONNFAILED;
}
if(send_greeting(handle)!=GREETING_OK) {
	uninitialize_handle(handle);
	return EGREETFAILED;
}
return HANDLE_OK;

This code works, and it's fairly clear what it does; but the problem
here is that you replicate deinitialization code. If that
deinitialization code ever needs to change, you have to change it in
multiple places. You might miss one, causing you a leak of sorts. It's
of course possible to nest the ifs, but that's even worse:

int handle = initialize_handle();
int retval = HANDLE_OK;
if(connect_handle(handle) != CONNECTION_OK) {
	retval = ECONNFAILED;
} else {
	if(send_greeting(handle) != GREETING_OK) {
		retval = EGREETFAILED;
	}
}
uninitialize_handle(handle);
return retval;

it's easy to see that this kind of code will become unreadable once
you're past three or four of these checks. However, the compiled code
will be *exactly* the same as the following:

int handle = initialize_handle();
int retval = HANDLE_OK;
if(connect_handle(handle) != CONNECTION_OK) {
	retval = ECONNFAILED;
	goto err_out;
}
if(send_greeting(handle) != GREETING_OK) {
	retval = EGREETFAILED;
	goto err_out;
}
err_out:
uninitialize_handle(handle);
return retval;

This is again as readable as the first example, but the deinitialization
is grouped at the bottom of the function.

-- 
Copyshops should do vouchers. So that next time some bureaucracy
requires you to mail a form in triplicate, you can mail it just once,
add a voucher, and save on postage.