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

From: "Alex G." <mr.nuke.me@gmail.com>
To: Simon Glass <sjg@chromium.org>,
	U-Boot Mailing List <u-boot@lists.denx.de>
Cc: Andre Przywara <andre.przywara@arm.com>,
	Rasmus Villemoes <rasmus.villemoes@prevas.dk>,
	Robert Marko <robert.marko@sartura.hr>,
	Masahiro Yamada <masahiroy@kernel.org>,
	Tom Rini <trini@konsulko.com>,
	Joe Hershberger <joe.hershberger@ni.com>,
	Marek Vasut <marex@denx.de>
Subject: Re: [PATCH v5 03/29] image: Add Kconfig options for FIT in the tools build
Date: Tue, 5 Oct 2021 13:17:40 -0500	[thread overview]
Message-ID: <4c1e0d13-2d90-a909-165e-b6197ddb2534@gmail.com> (raw)
In-Reply-To: <20210925194327.v5.3.I64826ed33219988294468df7b95dfa3fffd7a0a1@changeid>

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
> 