Re: [PATCH 2/2] mtd: nand: Get rid of needless 'goto'

[Andrey Smirnov]

On Tue, Jul 19, 2016 at 11:30 AM, Brian Norris
<computersforpeace@xxxxxxxxx> wrote:
> On Tue, Jul 19, 2016 at 08:41:44AM -0700, Andrey Smirnov wrote:
>> Using "goto" in that "switch" statement only makes it harder to follow
>> control flow and doesn't bring any advantages. Rewrite the code to avoid
>> using "goto".
>>
>> Signed-off-by: Andrey Smirnov <andrew.smirnov@xxxxxxxxx>
>> ---
>>  drivers/mtd/nand/nand_base.c | 13 +++++--------
>>  1 file changed, 5 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
>> index 57043a6..8fa5536 100644
>> --- a/drivers/mtd/nand/nand_base.c
>> +++ b/drivers/mtd/nand/nand_base.c
>> @@ -2139,18 +2139,15 @@ static int nand_read_oob(struct mtd_info *mtd, loff_t from,
>>       case MTD_OPS_PLACE_OOB:
>>       case MTD_OPS_AUTO_OOB:
>>       case MTD_OPS_RAW:
>> +             if (!ops->datbuf)
>> +                     ret = nand_do_read_oob(mtd, from, ops);
>> +             else
>> +                     ret = nand_do_read_ops(mtd, from, ops);
>>               break;
>> -
>>       default:
>> -             goto out;
>> +             break;
>>       }
>>
>> -     if (!ops->datbuf)
>> -             ret = nand_do_read_oob(mtd, from, ops);
>> -     else
>> -             ret = nand_do_read_ops(mtd, from, ops);
>> -
>> -out:
>>       nand_release_device(mtd);
>>       return ret;
>>  }
>
> The default case is really just a catch-all error case. We don't
> actually even need the nand_get_device() call for that... can we just
> do this instead?

Sure, although, if you don't mind, I'd rather you used:

if (ops->mode != MTD_OPS_PLACE_OOB &&
    ops->mode != MTD_OPS_AUTO_OOB &&
    ops->mode != MTD_OPS_RAW)
       return -ENOTSUPP;

instead of the switch statement, IMHO, this way it is more obvious
that this codepath is just arguments correctness checking.

Andrey