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

Brian Norris
2016-07-19T18:30:53Z

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@gmail.com>
> ---
> 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?

static int nand_read_oob(struct mtd_info *mtd, loff_t from,
struct mtd_oob_ops *ops)
{
int ret;

ops->retlen = 0;

/* Do not allow reads past end of device */
if (ops->datbuf && (from + ops->len) > mtd->size) {
pr_debug("%s: attempt to read beyond end of device\n",
__func__);
return -EINVAL;
}

switch (ops->mode) {
case MTD_OPS_PLACE_OOB:
case MTD_OPS_AUTO_OOB:
case MTD_OPS_RAW:
break;

default:
return -ENOTSUPP;
}

nand_get_device(mtd, FL_READING);

if (!ops->datbuf)
ret = nand_do_read_oob(mtd, from, ops);
else
ret = nand_do_read_ops(mtd, from, ops);

nand_release_device(mtd);
return ret;
}

i.e., this diff:

Signed-off-by: Brian Norris <computersforpeace@gmail.com>

diff --git a/drivers/mtd/nand/nand_base.c b/drivers/mtd/nand/nand_base.c
index 77533f7f2429..881dbd495466 100644
--- a/drivers/mtd/nand/nand_base.c
+++ b/drivers/mtd/nand/nand_base.c
@@ -2162,7 +2162,7 @@ static int nand_do_read_oob(struct mtd_info *mtd, loff_t from,
static int nand_read_oob(struct mtd_info *mtd, loff_t from,
struct mtd_oob_ops *ops)
{
- int ret = -ENOTSUPP;
+ int ret;

ops->retlen = 0;

@@ -2173,8 +2173,6 @@ static int nand_read_oob(struct mtd_info *mtd, loff_t from,
return -EINVAL;
}

- nand_get_device(mtd, FL_READING);
-
switch (ops->mode) {
case MTD_OPS_PLACE_OOB:
case MTD_OPS_AUTO_OOB:
@@ -2182,15 +2180,16 @@ static int nand_read_oob(struct mtd_info *mtd, loff_t from,
break;

default:
- goto out;
+ return -ENOTSUPP;
}

+ nand_get_device(mtd, FL_READING);
+
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;
}