Re: [PATCH RFC 02/11] libasm: Fix xdefault_pattern initialization

[Mark Wielaard <mark@klomp.org>]

Hi Ilya,

On Mon, Feb 06, 2023 at 11:25:04PM +0100, Ilya Leoshkevich via Elfutils-devel wrote:
> clang complains:
> 
>     asm_newscn.c:48:22: error: field 'pattern' with variable sized type 'struct FillPattern' not at the end of a struct or class is a GNU extension [-Werror,-Wgnu-variable-sized-type-not-at-end]
>       struct FillPattern pattern;
>                          ^
> 
> Fix by using a union instead. Define the second union member to be a
> char array 1 byte larger than struct FillPattern. This should be legal
> according to 6.7.9:
> 
>     If an object that has static or thread storage duration is not
>     initialized explicitly, then ... if it is a union, the first named
>     member is initialized (recursively) according to these rules, and
>     any padding is initialized to zero bits.
> 
> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> ---
>  libasm/asm_newscn.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/libasm/asm_newscn.c b/libasm/asm_newscn.c
> index d258d969..32a3b598 100644
> --- a/libasm/asm_newscn.c
> +++ b/libasm/asm_newscn.c
> @@ -43,17 +43,16 @@
>  /* Memory for the default pattern.  The type uses a flexible array
>     which does work well with a static initializer.  So we play some
>     dirty tricks here.  */
> -static const struct
> +static const union
>  {
>    struct FillPattern pattern;
> -  char zero;
> +  char zeroes[sizeof(struct FillPattern) + 1];
>  } xdefault_pattern =
>    {
>      .pattern =
>      {
>        .len = 1
>      },
> -    .zero = '\0'
>    };

Yes, I think this works. Could you update the comment just before this
with some of the commit message explanation? Your explanation is much
better than "play some dirty trick" :)

>  const struct FillPattern *__libasm_default_pattern = &xdefault_pattern.pattern;

I am surprised this doesn't need a cast. Do you know why?

Thanks,

Mark