Messages in this thread Patch in this message | | | Subject | Re: [PATCH v2 5/5] mtd: spi-nor: swp: Drop 'else' after 'return' | From | Joe Perches <> | Date | Sun, 14 Mar 2021 23:53:04 -0700 |
| |
On Mon, 2021-03-08 at 11:58 +0530, Pratyush Yadav wrote: > On 06/03/21 11:50AM, Tudor Ambarus wrote: > > else is not generally useful after a break or return. > > > > Signed-off-by: Tudor Ambarus <tudor.ambarus@microchip.com> > > Reviewed-by: Pratyush Yadav <p.yadav@ti.com> >
I don't think this improves the code.
Generally, checkpatch is a stupid little script.
This code uses a form like: if (foo) return bar; else return baz;
which checkpatch recognizes as OK and so checkpatch does not emit any warning message, but this code just adds comments before each return which confuses checkpatch.
I think better would be to change the code to use temporaries and convert the functions to bool.
Something like: --- drivers/mtd/spi-nor/core.c | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-)
diff --git a/drivers/mtd/spi-nor/core.c b/drivers/mtd/spi-nor/core.c index 0522304f52fa..e174a2f1d621 100644 --- a/drivers/mtd/spi-nor/core.c +++ b/drivers/mtd/spi-nor/core.c @@ -1798,36 +1798,41 @@ static void spi_nor_get_locked_range_sr(struct spi_nor *nor, u8 sr, loff_t *ofs, } /* - * Return 1 if the entire region is locked (if @locked is true) or unlocked (if - * @locked is false); 0 otherwise + * Return true if the entire region is locked + * (if @locked is true) or unlocked (if @locked is false); false otherwise */ -static int spi_nor_check_lock_status_sr(struct spi_nor *nor, loff_t ofs, +static bool spi_nor_check_lock_status_sr(struct spi_nor *nor, loff_t ofs, uint64_t len, u8 sr, bool locked) { loff_t lock_offs; uint64_t lock_len; + uint64_t lock_max; + uint64_t ofs_max; if (!len) - return 1; + return true; spi_nor_get_locked_range_sr(nor, sr, &lock_offs, &lock_len); + lock_max = lock_offs + lock_len; + ofs_max = ofs + len; + if (locked) /* Requested range is a sub-range of locked range */ - return (ofs + len <= lock_offs + lock_len) && (ofs >= lock_offs); + return (ofs_max <= lock_max) && (ofs >= lock_offs); else /* Requested range does not overlap with locked range */ - return (ofs >= lock_offs + lock_len) || (ofs + len <= lock_offs); + return (ofs >= lock_max) || (ofs_max <= lock_offs); } -static int spi_nor_is_locked_sr(struct spi_nor *nor, loff_t ofs, uint64_t len, - u8 sr) +static bool spi_nor_is_locked_sr(struct spi_nor *nor, loff_t ofs, uint64_t len, + u8 sr) { return spi_nor_check_lock_status_sr(nor, ofs, len, sr, true); } -static int spi_nor_is_unlocked_sr(struct spi_nor *nor, loff_t ofs, uint64_t len, - u8 sr) +static bool spi_nor_is_unlocked_sr(struct spi_nor *nor, loff_t ofs, uint64_t len, + u8 sr) { return spi_nor_check_lock_status_sr(nor, ofs, len, sr, false); }
| |