[U-Boot] [PATCH v3 4/4] mtdparts: new add.e: add part skipping bad blocks

[Wolfgang Denk]

Dear Ben Gardiner,

In message <1278366212-24023-5-git-send-email-bengardiner at nanometrics.ca> you wrote:
> This patch adds a new 'mtdparts add' variant: add.e (with a synomym add.i). This
> command variant adds a new partition to the mtdparts variable but also increases
> the partitions size by skipping bad blocks and aggregating any additional bad
> blocks found at the end of the partition.
> 
> Signed-off-by: Ben Gardiner <bengardiner at nanometrics.ca>
> 
> ---
> 
> V2:
>  * formatting: spaces after 'if' and 'for'
>  * trailing whitespace removed
> 
> V3:
>  * rebased to 54841ab50c20d6fa6c9cc3eb826989da3a22d934 of
>    git://git.denx.de/u-boot.git
>  * fix more checkpatch errors
>  * updating copyright to include addition of add.e command

NAK for this like for the previous patches.


> +#if defined(CONFIG_CMD_MTDPARTS_SPREAD)
> +		s = strchr(argv[1], '.');

s == NULL if there is no '.' in the argument.

> +		if (get_mtd_info(dev->id->type, dev->id->num, &mtd))
> +			return 1;
> +#endif
> +
>  		if ((dev_tmp = device_find(dev->id->type, dev->id->num)) == NULL) {
> +#if defined(CONFIG_CMD_MTDPARTS_SPREAD)
> +			if (!strcmp(s, ".e") || !strcmp(s, ".i")) {

So here you might dereference a NULL pointer. Sounds like a Bad Thing
to me.  Please add error checking / handling.

> +#if defined(CONFIG_CMD_MTDPARTS_SPREAD)
> +	"mtdparts add.e <mtd-dev> <size>[@<offset>] [<name>] [ro]\n"
> +	"    - add partition, padding size by skipping bad blocks\n"
> +#endif

Also document the ".i" version.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
I usually tell my classes "if you are using @ and [] together in this
class, you will almost certainly NOT get what you want. That's  going
down  the wrong tunnel. There's no cheese at the end of that tunnel."
         -- Randal L. Schwartz in <8czptuomey.fsf at gadget.cscaper.com>