Re: [review] slotinfo in struct dtv_slotinfo_list should be flexible array [BZ #25...

[Szabolcs Nagy <Szabolcs.Nagy@arm.com>]

On 03/11/2019 17:11, Florian Weimer (Code Review) wrote:
> Change URL: https://gnutoolchain-gerrit.osci.io/r/c/glibc/+/489

i can't login to gerrit. is there something special i need to do?

> slotinfo in struct dtv_slotinfo_list should be flexible array [BZ #25097]
> 
> GCC 10 will warn about subscribing inner length zero arrays.  Use a GCC
> extension in csu/libc-tls.c to allocate space for the static_slotinfo
> variable.  Adjust nptl_db so that the type description machinery does
> not attempt to determine the size of the flexible array member slotinfo.

the patch looks ok to me.

> -static struct
> -{
> -  struct dtv_slotinfo_list si;
> -  /* The dtv_slotinfo_list data structure does not include the actual
> -     information since it is defined as an array of size zero.  We define
> -     here the necessary entries.  Note that it is not important whether
> -     there is padding or not since we will always access the information
> -     through the 'si' element.  */
> -  struct dtv_slotinfo info[2 + TLS_SLOTINFO_SURPLUS];
> -} static_slotinfo;
> -
> +static struct dtv_slotinfo_list static_slotinfo =
> +  {
> +   /* Allocate an array of 2 + TLS_SLOTINFO_SURPLUS elements.  */
> +   .slotinfo =  { [array_length (_dl_static_dtv) - 1] = { } },
> +  };

i'd use {0} instead of {} as that's the universal initializer in c.

(to me the original code looked more obvious)

>  static void
>  init_slotinfo (void)
>  {
> -  /* Create the slotinfo list.  */
> -  static_slotinfo.si.len = (((char *) (&static_slotinfo + 1)
> -			     - (char *) &static_slotinfo.si.slotinfo[0])
> -			    / sizeof static_slotinfo.si.slotinfo[0]);
> -  // static_slotinfo.si.next = NULL;	already zero
> +  /* Create the slotinfo list.  Note that the type of static_slotinfo
> +     has effectively a zero-length array, so we cannot use the size of
> +     static_slotinfo to determine the array length.  */
> +  static_slotinfo.len = array_length (_dl_static_dtv);
> +  /* static_slotinfo.si.next = NULL; -- Already zero.  */

i'd remove the .si in the comment (or remove that comment)