[U-Boot] [PATCH] image-fit: Fix fit_get_node_from_config semantics

[Marek Vasut]

On 09/20/2016 07:17 PM, Paul Burton wrote:
> Commit bac17b78dace ("image-fit: switch ENOLINK to ENOENT") changed
> fit_get_node_from_config to return -ENOENT when a property doesn't
> exist, but didn't change any of its callers which check return values.
> Notably it didn't change boot_get_ramdisk, which leads to U-Boot failing
> to boot FIT images which don't include ramdisks with the following
> message:
> 
>   Ramdisk image is corrupt or invalid
> 
> It also didn't take into account that by returning -ENOENT to denote the
> lack of a property we lost the ability to determine from the return
> value of fit_get_node_from_config whether it was the property or the
> configuration node that was missing, which may potentially lead callers
> to accept invalid FIT images.
> 
> Fix this by having fit_get_node_from_config return -EINVAL when the
> configuration node isn't found and -ENOENT when the property isn't
> found, which seems to make semantic sense. Callers that previously
> checked for -ENOLINK are adjusted to check for -ENOENT, which fixes the
> breakage introduced by commit bac17b78dace ("image-fit: switch ENOLINK
> to ENOENT").
> 
> The only other user of the return fit_get_node_from_config return value,
> indirectly, is bootm_find_os which already checked for -ENOENT. From a
> read-through of the code I suspect it ought to have been checking for
> -ENOLINK prior to bac17b78dace ("image-fit: switch ENOLINK to ENOENT")
> anyway, which would make it right after this patch, but this would be
> good to get verified by someone who knows this x86 code or is able to
> test it.
> 
> Signed-off-by: Paul Burton <paul.burton at imgtec.com>
> Cc: Jonathan Gray <jsg at jsg.id.au>
> Cc: Marek Vasut <marex at denx.de>

Acked-by: Marek Vasut <marex at denx.de>

> 
> ---
> 
>  common/image-fdt.c | 2 +-
>  common/image-fit.c | 2 +-
>  common/image.c     | 2 +-
>  3 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/common/image-fdt.c b/common/image-fdt.c
> index d6ee225..3d23608 100644
> --- a/common/image-fdt.c
> +++ b/common/image-fdt.c
> @@ -285,7 +285,7 @@ int boot_get_fdt(int flag, int argc, char * const argv[], uint8_t arch,
>  			fdt_noffset = fit_get_node_from_config(images,
>  							       FIT_FDT_PROP,
>  							       fdt_addr);
> -			if (fdt_noffset == -ENOLINK)
> +			if (fdt_noffset == -ENOENT)
>  				return 0;
>  			else if (fdt_noffset < 0)
>  				return 1;
> diff --git a/common/image-fit.c b/common/image-fit.c
> index 9ce68f1..1b0234a 100644
> --- a/common/image-fit.c
> +++ b/common/image-fit.c
> @@ -1560,7 +1560,7 @@ int fit_get_node_from_config(bootm_headers_t *images, const char *prop_name,
>  	cfg_noffset = fit_conf_get_node(fit_hdr, images->fit_uname_cfg);
>  	if (cfg_noffset < 0) {
>  		debug("*  %s: no such config\n", prop_name);
> -		return -ENOENT;
> +		return -EINVAL;
>  	}
>  
>  	noffset = fit_conf_get_prop_node(fit_hdr, cfg_noffset, prop_name);
> diff --git a/common/image.c b/common/image.c
> index 7ad04ca..c8d9bc8 100644
> --- a/common/image.c
> +++ b/common/image.c
> @@ -1078,7 +1078,7 @@ int boot_get_ramdisk(int argc, char * const argv[], bootm_headers_t *images,
>  			rd_addr = map_to_sysmem(images->fit_hdr_os);
>  			rd_noffset = fit_get_node_from_config(images,
>  					FIT_RAMDISK_PROP, rd_addr);
> -			if (rd_noffset == -ENOLINK)
> +			if (rd_noffset == -ENOENT)
>  				return 0;
>  			else if (rd_noffset < 0)
>  				return 1;
> 


-- 
Best regards,
Marek Vasut