[RFC PATCH 1/4] common/image.c: image_decomp: put IH_COMP_XXX cases inside ifndef USE_HOSTCC

[Tom Rini]

On Wed, Jun 17, 2020 at 09:07:20AM +0200, Rasmus Villemoes wrote:
> On 17/06/2020 05.11, Simon Glass wrote:
> > Hi Rasmus,
> > 
> > On Fri, 12 Jun 2020 at 05:02, Rasmus Villemoes
> > <rasmus.villemoes at prevas.dk> wrote:
> >>
> >> When building host tools, the CONFIG_GZIP etc. symbols are not defined
> >> anyway, so this does not (should not) change anything [1]. However,
> >> since the host tools also don't include linux/kconfig.h, one cannot
> >> use the CONFIG_IS_ENABLED() smartness in a preprocessor conditional,
> >> which in turn prevents one from adding, say, an
> >>
> >>   #if CONFIG_IS_ENABLED(ZSTD)
> >>
> >> case.
> >>
> >> OTOH, with this, one can do that, and it also makes it possible to
> >> convert say, "#ifdef CONFIG_GZIP" to "#if CONFIG_IS_ENABLED(GZIP)" to
> >> make those other cases SPL-or-not-SPL-aware.
> >>
> >> [1] The gzip.h header is only included under !USE_HOSTCC, and removing
> >> the CONFIG_GZIP conditional hence both gives an "no declaration of
> >> gunzip" warning as well as breaks the link of the host tools, since
> >> the gunzip code is indeed not linked in.
> >>
> >> Signed-off-by: Rasmus Villemoes <rasmus.villemoes at prevas.dk>
> >> ---
> >>  common/image.c | 2 ++
> >>  1 file changed, 2 insertions(+)
> >>
> >> diff --git a/common/image.c b/common/image.c
> >> index e1ca1a7905..73f6845274 100644
> >> --- a/common/image.c
> >> +++ b/common/image.c
> >> @@ -458,6 +458,7 @@ int image_decomp(int comp, ulong load, ulong image_start, int type,
> >>                 else
> >>                         ret = -ENOSPC;
> >>                 break;
> >> +#ifndef USE_HOSTCC
> > 
> > In general I'm not a fan of the USE_HOSTCC stuff. I suspect HOST
> > should be a Kconfig like anything else, although perhaps magically
> > inserted somehow. Then we can do more things at compile-time.
> 
> Yes, as you can see in 2/4, I agree that the hostcc stuff is messy. I
> imagine we can introduce a Kconfig.hosttools at some point which can
> have a lot of def_bool y symbols (and of course one could eventually
> allow them to be actually choosable, e.g. to allow the user to not build
> some tool or functionality that he doesn't use and which requires some
> external library dependency).
> 
> So for example, to make the fit_check_sign actually also check the
> decompression, we'd add a
> 
> CONFIG_HOSTTOOL_GZIP
>   def_bool y
> 
> move this USE_HOSTCC a bit further down, and change the #ifdef
> CONFIG_GZIP to #if CONFIG_IS_ENABLED(GZIP) - all after checking all the
> other places CONFIG_GZIP is tested... so not something that's just done
> mechanically or over night, but I think it can be done mostly piecemeal.

I agree, we can improve this area piecemeal.  We need to perhaps give a
harder think to what code should be shared directly and where it's more
of a pain than it's worth too.

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20200617/eb46c62c/attachment.sig>