Re: [PATCH] xen/decompress: make helper symbols static

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 18/01/2021 15:13, Jan Beulich wrote:
> On 18.01.2021 12:58, Andrew Cooper wrote:
>> On 18/01/2021 11:19, Jan Beulich wrote:
>>> The individual decompression CUs need to only surface their top level
>>> functions to other code. Arrange for everything else to be static, to
>>> make sure no undue uses of that code exist or will appear without
>>> explicitly noticing. (In some cases this also results in code size
>>> reduction, but since this is all init-only code this probably doesn't
>>> matter very much.)
>>>
>>> In the LZO case also take the opportunity and convert u8 where lines
>>> get touched anyway.
>>>
>>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>>> ---
>>> The downside is that the top level functions will now be non-static
>>> in stubdom builds of libxenguest, but I think that's acceptable.
>> Yeah - not something to lose sleep over.
>>
>> Acked-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> Thanks, but this actually breaks the stubdom build, which now sees
> non-static function definitions without prior declaration, which
> with -Werror results in the build to fail. There's this extra hunk
> now which addresses this:
>
> --- unstable.orig/tools/libs/guest/xg_dom_decompress_unsafe.h
> +++ unstable/tools/libs/guest/xg_dom_decompress_unsafe.h
> @@ -1,8 +1,12 @@
> +#ifdef __MINIOS__
> +# include "../../xen/include/xen/decompress.h"
> +#else
>  typedef int decompress_fn(unsigned char *inbuf, unsigned int len,
>                            int (*fill)(void*, unsigned int),
>                            int (*flush)(void*, unsigned int),
>                            unsigned char *outbuf, unsigned int *posp,
>                            void (*error)(const char *x));
> +#endif
>  
>  int xc_dom_decompress_unsafe(
>      decompress_fn fn, struct xc_dom_image *dom, void **blob, size_t *size)
>
> i.e. strictly speaking I may then also need a tools side ack.

My ack still stands.

>
>> but I really would
>> like to see the STATIC and INIT defines disappear eventually.
> I too would like to, but I don't see this happen in particular
> for INIT and INITDATA, when we have two distinct environments
> where this code gets built. Unless you envision the tool stack /
> stubdom side of the build to gain
>
> #define __init
> #define __initdata
>
> ?

I think that's fine.

One way or another, some header file somewhere needs to plumb this up
for the differing environments.  I just don't see the need for an extra
level of indirection.

> As far as STATIC goes, being in the middle of the DomU side
> work of this, I've found a need to sprinkle around quite a
> few of them in zstd/decompress.c, again to silence similar
> compiler diagnostics.

Do you mean you've had to add more STATIC's in?

~Andrew