[U-Boot] [PATCH] Followup fixes on the mtdparts spread patchset

[Ben Gardiner]

Hi Scott,

Thank you for picking up the 'mtdparts spread' patch series into
u-boot-nand-flash next by applying the v6 series and doing this fixup
on-top. It is greatly appreciated.

On Thu, Sep 9, 2010 at 4:50 PM, Scott Wood <scottwood at freescale.com> wrote:
> Consolidate some code in mtd_get_len_incl_bad(), and fix a condition
> where a valid partition could be reported as truncated if it has a
> good block at the end of the device (unlikely, since the BBT is usually
> there).
>
> Fix mid-block declarations in net_part_size().
>
> Signed-off-by: Scott Wood <scottwood at freescale.com>
> ---
>  common/cmd_mtdparts.c |    5 +++--
>  drivers/mtd/mtdcore.c |   15 +++++----------
>  2 files changed, 8 insertions(+), 12 deletions(-)
>
> diff --git a/common/cmd_mtdparts.c b/common/cmd_mtdparts.c
> index 17865b7..5481c88 100644
> --- a/common/cmd_mtdparts.c
> +++ b/common/cmd_mtdparts.c
> @@ -1228,15 +1228,16 @@ static int generate_mtdparts_save(char *buf, u32 buflen)
>  */
>  static uint64_t net_part_size(struct mtd_info *mtd, struct part_info *part)
>  {
> +       uint64_t i, net_size = 0;
> +
>        if (!mtd->block_isbad)
>                return part->size;
>
> -       uint64_t i, net_size = 0;
> -
>        for (i = 0; i < part->size; i += mtd->erasesize) {
>                if (!mtd->block_isbad(mtd, part->offset + i))
>                        net_size += mtd->erasesize;
>        }
> +
>        return net_size;
>  }
>  #endif

It's fine by me but I'm curious: why move the declaration of the stack
variables i and net_size to before the check of the block_isbad
function pointer? Before this change, I think there would be no
allocation of these variables on the stack if the block_isbad function
pointer is null.

I double-checked and there were no warnings from 'make' 'MAKEALL' (I'm
using (Sourcery G++ Lite 2009q1-203) 4.3.3 here) or checkpatch.pl
(from 2.6.36-rc3)  before this change. Just curious -- I am interested
in learning so I can produce and submit patchsets that meet the
standards of this list in the future.

> diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
> index 78f2a08..a195dda 100644
> --- a/drivers/mtd/mtdcore.c
> +++ b/drivers/mtd/mtdcore.c
> @@ -162,11 +162,6 @@ void mtd_get_len_incl_bad(struct mtd_info *mtd, uint64_t offset,
>        *truncated = 0;
>        *len_incl_bad = 0;
>
> -       if (offset >= mtd->size) {
> -               *truncated = 1;
> -               return;
> -       }
> -
>        if (!mtd->block_isbad) {
>                *len_incl_bad = length;
>                return;
> @@ -176,6 +171,11 @@ void mtd_get_len_incl_bad(struct mtd_info *mtd, uint64_t offset,
>        uint64_t block_len;
>
>        while (len_excl_bad < length) {
> +               if (offset >= mtd->size) {
> +                       *truncated = 1;
> +                       return;
> +               }
> +
>                block_len = mtd->erasesize - (offset & (mtd->erasesize - 1));
>
>                if (!mtd->block_isbad(mtd, offset & ~(mtd->erasesize - 1)))
> @@ -183,11 +183,6 @@ void mtd_get_len_incl_bad(struct mtd_info *mtd, uint64_t offset,
>
>                *len_incl_bad += block_len;
>                offset       += block_len;
> -
> -               if (offset >= mtd->size) {
> -                       *truncated = 1;
> -                       break;
> -               }
>        }
>  }
>  #endif /* defined(CONFIG_CMD_MTDPARTS_SPREAD) */

Awesome! Thank you for fixing that edge case.

Reviewed-by: Ben Gardiner <bengardiner at nanometrics.ca>

Best Regards,
Ben Gardiner

---
Nanometrics Inc.
http://www.nanometrics.ca