Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1186733

Re: [PATCH 3/6] staging: rtl8188eu: remove goto label

Show all headers | View raw

On Fri, Jul 17, 2015 at 04:55:12PM +0530, Sudip Mukherjee wrote:
> On Fri, Jul 17, 2015 at 02:03:48PM +0300, Dan Carpenter wrote:
> > On Thu, Jul 16, 2015 at 04:58:09PM +0530, Sudip Mukherjee wrote:
> > > By checking for the success of kzalloc we were able to remove the goto
> > > label thus making the code more readable.
> > > 
> > 
> > No...  You've just changed error handling to success handling and added
> > some new indent levels and made a tangled spaghetti exit path even more
> > tangled.  Spoderman wants to know, "Why u do dis?"
> I had to go for google to lookup Spoderman or Spiderman. :)
> At the end of the series, isn't the code better looking and simpler
> than the original code?

Yes, but that's not a very high bar to clear.

What I'm trying to explain is the beauty of writing one statement after
the other without indenting.  The simplest kind of code looks like this
without error handling.


int some_function(void)
{
	do_thing_one();
	do_thing_two();
	do_thing_three();

	return 0;
}

When you add canonical error handling it looks like this:

int some_function(void)
{
	do_thing_one();
	if (failure)
		return -ERROR;

	do_thing_two();
	if (failure)
		goto undo_one;

	do_thing_three();
	if (failure)
		goto undo_two;

	return 0;

undo_two:
	undo_two_thing();
undo_one:
	undo_one_thing();

	return -ERROR;
}

But with success handling it look like this:

int some_function(void)
{
	do_thing_one();
	if (success) {
		do_thing_two();
		if (success) {
			do_thing_three();
			if (success)
				return 0;
		}
	}
	return -ERROR;
}

Fewer lines, but terrible code.  In this patch we only make the last
condition success handling but it's still messier than need be.

	if (fail)
	if (fail)
	if (fail)
	if (success)
	else

The canonical method is more consistent and simplest to read.

regards,
dan carpenter
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH 3/6] staging: rtl8188eu: remove goto label Sudip Mukherjee <sudipm.mukherjee@gmail.com> - 2015-07-16 13:30 +0200
  Re: [PATCH 3/6] staging: rtl8188eu: remove goto label Dan Carpenter <dan.carpenter@oracle.com> - 2015-07-17 13:10 +0200
    Re: [PATCH 3/6] staging: rtl8188eu: remove goto label Sudip Mukherjee <sudipm.mukherjee@gmail.com> - 2015-07-17 13:30 +0200
      Re: [PATCH 3/6] staging: rtl8188eu: remove goto label Dan Carpenter <dan.carpenter@oracle.com> - 2015-07-17 13:50 +0200
        Re: [PATCH 3/6] staging: rtl8188eu: remove goto label Sudip Mukherjee <sudipm.mukherjee@gmail.com> - 2015-07-17 14:10 +0200

csiph-web