Hi Marek,

On 18/09/16 14:27, Marek Vasut wrote:
> This patch broke booting of any fitImage-wrapped kernel images due
> to replacement of ENOLINK with ENOENT without checking where the
> ENOLINK return value is being tested for. Adjust the tests as well
> to repair the breakage.

It's not obvious from the commit message what "this patch" refers to - I
think it would be useful to include the hash & subject of the broken
commit, eg:

  Commit bac17b78dace ("image-fit: switch ENOLINK to ENOENT") broke...

The (potential?) problem with the approach you took in this patch is
that the return value from fit_get_node_from_config no longer
differentiates between the ramdisk property of a FIT config being
missing (-ENOLINK prior to bac17b78dace) & the configuration itself
being missing (-ENOENT). Could that possibly lead to accepting invalid
FIT images? If not then I imagine it's by some convoluted chance & that
we'd probably be best to not rely on it.

If using ENOLINK isn't an option then my suggestion would be that we
change the config not found case to return -EINVAL & the property not
found can keep returning -ENOENT. Semantically that would seem to make
sense but it would mean checking all the callers, direct or indirect, of
fit_get_node_from_config to see whether they rely upon -ENOENT for the
config not found case.

Thanks,
    Paul

> 
> Signed-off-by: Marek Vasut <ma...@denx.de>
> Cc: Jonathan Gray <j...@jsg.id.au>
> Cc: Paul Burton <paul.bur...@imgtec.com>
> Cc: Tom Rini <tr...@konsulko.com>
> ---
>  common/image-fdt.c | 2 +-
>  common/image.c     | 2 +-
>  2 files changed, 2 insertions(+), 2 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.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;
> 
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot

Reply via email to