linux-kernel - Re: [PATCH v2] ARM: imx: fix error handling

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: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Sat, 17 May 2014 01:21:08 +0300
From:	Dan Carpenter <dan.carpenter@...cle.com>
To:	Uwe Kleine-König 
	<u.kleine-koenig@...gutronix.de>
Cc:	Emil Goode <emilgoode@...il.com>,
	Shawn Guo <shawn.guo@...escale.com>,
	Sascha Hauer <kernel@...gutronix.de>,
	Russell King <linux@....linux.org.uk>,
	linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
	kernel-janitors@...r.kernel.org
Subject: Re: [PATCH v2] ARM: imx: fix error handling

On Fri, May 16, 2014 at 09:24:51PM +0200, Uwe Kleine-König wrote:
> I didn't check if it is easily possible, but converting this file to use
> platform_device_register_full might simplify it considerably.

In a separate patch, though, please.

> 
> I'm not sure this fix is critical, because the problem happens if an
> allocation during boot fails. But still, if you want to get this fix
> into a stable release, you should simplify it, i.e. don't do the code
> reorganisations. (Also the "more clear" part seems to be subjective, I
> like the error handling better as it is now. But that might only be me.)

Emil's error handling is done exactly in the correct way...  The error
path and success path are separate.  Unwinding in the reverse order.
The label names describe the label locations.  Most days I spend hours
looking at linux kernel error handling and I can assure you that
sensible labels like this are a rare and wonderful gift.

It's hard for me to imagine how anyone could defend the original error
handling.  The label was "err".  The error handling was randomly plopped
in the middle of the success handling.  Whenever I see new "creative"
error handling like this it drives me nuts because obviously it's going
to be buggy like a swamp picnic.

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/