Re: [PATCH 0/2] nand: Remove BUG abuse

Ezequiel Garcia
2016-04-04T15:39:52Z

On 4 April 2016 at 12:33, Boris Brezillon
<boris.brezillon@free-electrons.com> wrote:
> On Sat, 2 Apr 2016 12:37:06 -0300
> Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> wrote:
>
>> On 4 April 2016 at 12:20, Boris Brezillon
>> <boris.brezillon@free-electrons.com> wrote:
>> > On Sat, 2 Apr 2016 15:55:24 +0200
>> > Boris Brezillon <boris.brezillon@free-electrons.com> wrote:
>> >
>> >> On Fri, 1 Apr 2016 18:29:24 -0300
>> >> Ezequiel Garcia <ezequiel@vanguardiasur.com.ar> wrote:
>> >>
>> >> > There's no reason to BUG() when parameters are being
>> >> > validated. Drivers can get things wrong, and it's much nicer
>> >> > to just throw a noisy warn and fail gracefully, than calling
>> >> > BUG() and throwing the whole system down the drain.
>> >>
>> >> I'm fine with this change as long as all callers are checking
>> >> nand_scan_tail() return value.
>> >
>> > Actually, the s3c2410 driver is not checking nand_scan_tail() return
>> > value. Could you send a v2 addressing that?
>> >
>>
>> Hmm, I don't see how that relates to this patch.
>> As far as I can see, it's two completely independent issues.
>>
>> Or am I missing something here?
>
> Well, you're removing BUG() calls and are returning an error instead, so
> if existing nand_scan_tail() callers don't check the return status you
> may hide an existing bug...
>

Yes, I am removing a BUG() - thus replacing an oops (potentially a panic),
with an error return.

However, nand_scan_tail can already fail and return an error
for several other reasons.

> I know it's unlikely to happen, but I'd still prefer to have all
> nand_scan_tail() callers to check the return value before removing
> those calls to BUG().
>

It's not about unlikeliness, but about correlation and bisectability.
IMO, the fact that some drivers don't check the return of nand_scan_tail
is a bug on its own, and has nothing to do with this patch.
--
Ezequiel García, VanguardiaSur