lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:	Wed, 6 Nov 2013 12:30:50 +0300
From:	Dan Carpenter <dan.carpenter@...cle.com>
To:	Kishon Vijay Abraham I <kishon@...com>
Cc:	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	linux-kernel@...r.kernel.org, kernel-janitors@...r.kernel.org,
	Tomasz Figa <t.figa@...sung.com>, Felipe Balbi <balbi@...com>,
	Sylwester Nawrocki <s.nawrocki@...sung.com>
Subject: Re: [patch] drivers: phy: tweaks to phy_create()

On Wed, Nov 06, 2013 at 01:41:26PM +0530, Kishon Vijay Abraham I wrote:

> >The rest of this patch is just cleanup like returning directly instead
> >of having do-nothing gotos.  Using descriptive labels instead of
> 
> Grouping the err returns in the end looked a bit cleaner to me. It's
> just a matter of preference I guess.

Say if you are reading the code and it says:

	goto err0;

What you think is, "Huh, that's a crap name for a label.  I wonder what
it does?"  So you have interrupt what you are doing and scroll down and
then you see that it doesn't do anything.  Not a single thing.  It just
returns.  Then you have to scroll back and try remember where you were.

But if you use a clear label like:

	ret = -ENOMEM;
	goto return_ret;

Then I don't have to scroll down to find out what it is.

But it's even more clear to just say:

	return -ENOMEM;

The original code was not just messy, it was also buggy.  If you do the
error handling in the right way by unwinding in the correct order and
thinking about label names then your code will be less buggy.

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

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ