Re: [PATCH v5 03/29] image: Add Kconfig options for FIT in the tools build

Alex G.
2021-10-05T18:17:44Z

On 9/25/21 8:43 PM, Simon Glass wrote:
> In preparation for enabling CONFIG_IS_ENABLED() on the host build, add
> some options to enable the various FIT options expected in these tools.
> This will ensure that the code builds correctly when CONFIG_TOOLS_xxx
> is distinct from CONFIG_xxx.
>
> Drop some #ifdefs which are immediately unnecessary (many more are in
> later patches).
>
> Signed-off-by: Simon Glass <sjg@chromium.org>

Under protest,
Reviewed-by: Alexandru Gagniuc <mr.nuke.me@gmail.com>
> ---
>
> Changes in v5:
> - Drop changes to tools/Makefile since they already use TOOLS_ Kconfigs
> - Use TOOLS_ instead of HOST_
>
> common/image-fit-sig.c | 3 ++-
> common/image-fit.c | 4 ++--
> tools/Kconfig | 25 +++++++++++++++++++++++++
> 3 files changed, 29 insertions(+), 3 deletions(-)
>
> diff --git a/common/image-fit-sig.c b/common/image-fit-sig.c
> index b979cd2a4b6..e95e64bd2fe 100644
> --- a/common/image-fit-sig.c
> +++ b/common/image-fit-sig.c
> @@ -72,11 +72,12 @@ static int fit_image_setup_verify(struct image_sign_info *info,
> char *algo_name;
> const char *padding_name;
>
> +#ifndef USE_HOSTCC
> if (fdt_totalsize(fit) > CONFIG_FIT_SIGNATURE_MAX_SIZE) {

If you set CONFIG_FIT_SIGNATURE_MAX_SIZE to 0x10000000, then this
comparison will always be false.

> *err_msgp = "Total size too large";
> return 1;
> }
> -
> +#endif
> if (fit_image_hash_get_algo(fit, noffset, &algo_name)) {
> *err_msgp = "Can't get hash algo property";
> return -1;
> diff --git a/common/image-fit.c b/common/image-fit.c
> index 6f8e67e4158..17c6d4e7813 100644
> --- a/common/image-fit.c
> +++ b/common/image-fit.c
> @@ -170,7 +170,7 @@ int fit_get_subimage_count(const void *fit, int images_noffset)
> return count;
> }
>
> -#if CONFIG_IS_ENABLED(FIT_PRINT) || CONFIG_IS_ENABLED(SPL_FIT_PRINT)
> +#if CONFIG_IS_ENABLED(FIT_PRINT)
> /**
> * fit_image_print_data() - prints out the hash node details
> * @fit: pointer to the FIT format image header
> @@ -578,7 +578,7 @@ void fit_image_print(const void *fit, int image_noffset, const char *p)
> #else
> void fit_print_contents(const void *fit) { }
> void fit_image_print(const void *fit, int image_noffset, const char *p) { }
> -#endif /* CONFIG_IS_ENABLED(FIR_PRINT) || CONFIG_IS_ENABLED(SPL_FIT_PRINT) */
> +#endif /* CONFIG_IS_ENABLED(FIT_PRINT) */
>
> /**
> * fit_get_desc - get node description property
> diff --git a/tools/Kconfig b/tools/Kconfig
> index d6f82cd949b..ea986ab0479 100644
> --- a/tools/Kconfig
> +++ b/tools/Kconfig
> @@ -20,4 +20,29 @@ config TOOLS_LIBCRYPTO
> This selection does not affect target features, such as runtime FIT
> signature verification.
>
> +config TOOLS_FIT
> +config TOOLS_FIT_FULL_CHECK
> +config TOOLS_FIT_PRINT
> +config TOOLS_FIT_SIGNATURE

Besides FIT_PRINT, I'm confused as to where the others are used by the
tools build.

> +
> +config TOOLS_FIT_SIGNATURE_MAX_SIZE

On the other hand, if you #ifdef out the check in image-fit-sig.c, then
it doesn't make sense to have this config on the tools.

> + hex
> + depends on TOOLS_FIT_SIGNATURE
> + default 0x10000000
> +
> endmenu
>