From mboxrd@z Thu Jan  1 00:00:00 1970
From: Scott Wood <oss@buserror.net>
Date: Mon, 15 Aug 2016 11:48:31 -0500
Subject: [U-Boot] [PATCH v2] common: env_nand: Ensure that we have
 nand_info[0] prior to use
In-Reply-To: <1471276604-25544-1-git-send-email-trini@konsulko.com>
References: <1471276604-25544-1-git-send-email-trini@konsulko.com>
Message-ID: <1471279711.25630.270.camel@buserror.net>
List-Id: <u-boot.lists.denx.de>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: u-boot@lists.denx.de

On Mon, 2016-08-15 at 11:56 -0400, Tom Rini wrote:
> Now that nand_info[] is an array of pointers we need to ensure that it's
> been populated prior to use.??We may for example have ENV in NAND set in
> configurations that run on boards with and without NAND (where default
> env is fine enough, such as omap3_beagle and beagleboard (NAND) vs
> beagle xM (no NAND)).
> 
> Fixes: b616d9b0a708 ("nand: Embed mtd_info in struct nand_chip")
> Cc: Scott Wood <oss@buserror.net>
> Signed-off-by: Tom Rini <trini@konsulko.com>
> ---
> Changes in v2:
> - Oops, move check on the saveenv side in to erase_and_write_env
> ---
> ?common/env_nand.c | 9 +++++++--
> ?1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/common/env_nand.c b/common/env_nand.c
> index fc99a5e3fc0d..96a1020b5e79 100644
> --- a/common/env_nand.c
> +++ b/common/env_nand.c
> @@ -163,6 +163,9 @@ static int erase_and_write_env(const struct env_location
> *location,
> ?{
> ?	int ret = 0;
> ?
> +	if (!nand_info[0])
> +		return 1;
> +
> ?	printf("Erasing %s...\n", location->name);
> ?	if (nand_erase_opts(nand_info[0], &location->erase_opts))
> ?		return 1;
> @@ -244,10 +247,12 @@ static int readenv(size_t offset, u_char *buf)
> ?{
> ?	size_t end = offset + CONFIG_ENV_RANGE;
> ?	size_t amount_loaded = 0;
> -	size_t blocksize, len;
> +	size_t blocksize = 0, len;
> ?	u_char *char_ptr;
> ?
> -	blocksize = nand_info[0]->erasesize;
> +	if (nand_info[0])
> +		blocksize = nand_info[0]->erasesize;
> +
> ?	if (!blocksize)
> ?		return 1;
> ?

Messing around with blocksize rather than directly returning is awkward -- it
makes it look as if you could proceed with blocksize zero until you read the
next statement. ?And is there ever a case where blocksize can be zero, now
that uninitialized NAND devices will simply have a NULL pointer?

-Scott