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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Wouter,
On Sat, Mar 30, 2013 at 08:23:45AM +0100, Wouter Verhelst wrote:
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.

Thanks for the detailed example, do you want to teach me to be a kernel
hacker? :-)

I don't know too much C, but I have the feeling that in your last example
uninitialize_handle is called in any case, or are commands after the goto
lable skipped if you do not jump to them? If that is the case, I do not find
this more readable. I think the last lines of your example have to look like
this:

goto no_err;
err_out:
uninitialize_handle(handle);
no_err:
return retval;

And that in my exes is spaghetti... but I understand the rest of your
reasoning.

bon appetit!
Christian
--
To unsubscribe from this list: send the line "unsubscribe linux-m68k" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Video for Linux]     [Yosemite News]     [Linux S/390]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux