[14/26] locale: Remove private union from struct __locale_data

  This avoids an alias violation later.  This commit also fixes
an incorrect double-checked locking idiom in _nl_init_era_entries.
---
 locale/C-address.c        |  2 +-
 locale/C-collate.c        |  2 +-
 locale/C-ctype.c          |  2 +-
 locale/C-identification.c |  2 +-
 locale/C-measurement.c    |  2 +-
 locale/C-messages.c       |  2 +-
 locale/C-monetary.c       |  2 +-
 locale/C-name.c           |  2 +-
 locale/C-numeric.c        |  2 +-
 locale/C-paper.c          |  2 +-
 locale/C-telephone.c      |  2 +-
 locale/C-time.c           |  2 +-
 locale/localeinfo.h       | 14 ++++++------
 time/alt_digit.c          | 47 ++++++++++++++++----------------------
 time/era.c                | 48 +++++++++++++++++++--------------------
 time/lc-time-cleanup.c    |  4 ++--
 wcsmbs/wcsmbsload.c       | 10 ++++----
 wcsmbs/wcsmbsload.h       |  8 ++++---
 18 files changed, 74 insertions(+), 81 deletions(-)
  

On 17/03/2022 16:30, Florian Weimer via Libc-alpha wrote:
> This avoids an alias violation later.  This commit also fixes
> an incorrect double-checked locking idiom in _nl_init_era_entries.

LGTM, thanks.

Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>

> ---
>  locale/C-address.c        |  2 +-
>  locale/C-collate.c        |  2 +-
>  locale/C-ctype.c          |  2 +-
>  locale/C-identification.c |  2 +-
>  locale/C-measurement.c    |  2 +-
>  locale/C-messages.c       |  2 +-
>  locale/C-monetary.c       |  2 +-
>  locale/C-name.c           |  2 +-
>  locale/C-numeric.c        |  2 +-
>  locale/C-paper.c          |  2 +-
>  locale/C-telephone.c      |  2 +-
>  locale/C-time.c           |  2 +-
>  locale/localeinfo.h       | 14 ++++++------
>  time/alt_digit.c          | 47 ++++++++++++++++----------------------
>  time/era.c                | 48 +++++++++++++++++++--------------------
>  time/lc-time-cleanup.c    |  4 ++--
>  wcsmbs/wcsmbsload.c       | 10 ++++----
>  wcsmbs/wcsmbsload.h       |  8 ++++---
>  18 files changed, 74 insertions(+), 81 deletions(-)
> 
> diff --git a/locale/C-address.c b/locale/C-address.c
> index 1f509e4785..40ce9fbcf3 100644
> --- a/locale/C-address.c
> +++ b/locale/C-address.c
> @@ -26,7 +26,7 @@ const struct __locale_data _nl_C_LC_ADDRESS attribute_hidden =
>  {
>    _nl_C_name,
>    NULL, 0, 0,			/* no file mapped */
> -  { NULL, },			/* no cached data */
> +  NULL,				/* No cached data.  */
>    UNDELETABLE,
>    0,
>    13,
> diff --git a/locale/C-collate.c b/locale/C-collate.c
> index 510e90cf14..f9c2b7741b 100644
> --- a/locale/C-collate.c
> +++ b/locale/C-collate.c
> @@ -25,7 +25,7 @@ const struct __locale_data _nl_C_LC_COLLATE attribute_hidden =
>  {
>    _nl_C_name,
>    NULL, 0, 0,			/* no file mapped */
> -  { NULL, },			/* no cached data */
> +  NULL,				/* No cached data.  */
>    UNDELETABLE,
>    0,
>    19,
> diff --git a/locale/C-ctype.c b/locale/C-ctype.c
> index cc99bff930..ef4b67029c 100644
> --- a/locale/C-ctype.c
> +++ b/locale/C-ctype.c
> @@ -542,7 +542,7 @@ const struct __locale_data _nl_C_LC_CTYPE attribute_hidden =
>  {
>    _nl_C_name,
>    NULL, 0, 0,			/* no file mapped */
> -  { NULL, },			/* no cached data */
> +  NULL,				/* No cached data.  */
>    UNDELETABLE,
>    1,		/* Enable transliteration by default.  */
>    NR_FIXED + NR_CLASSES + NR_MAPS,
> diff --git a/locale/C-identification.c b/locale/C-identification.c
> index edaf417c70..9648de05bc 100644
> --- a/locale/C-identification.c
> +++ b/locale/C-identification.c
> @@ -26,7 +26,7 @@ const struct __locale_data _nl_C_LC_IDENTIFICATION attribute_hidden =
>  {
>    _nl_C_name,
>    NULL, 0, 0,			/* no file mapped */
> -  { NULL, },			/* no cached data */
> +  NULL,				/* No cached data.  */
>    UNDELETABLE,
>    0,
>    16,
> diff --git a/locale/C-measurement.c b/locale/C-measurement.c
> index b98d624b16..99e16caa3e 100644
> --- a/locale/C-measurement.c
> +++ b/locale/C-measurement.c
> @@ -26,7 +26,7 @@ const struct __locale_data _nl_C_LC_MEASUREMENT attribute_hidden =
>  {
>    _nl_C_name,
>    NULL, 0, 0,			/* no file mapped */
> -  { NULL, },			/* no cached data */
> +  NULL,				/* No cached data.  */
>    UNDELETABLE,
>    0,
>    2,
> diff --git a/locale/C-messages.c b/locale/C-messages.c
> index 88331c7ab6..8fc5c397e3 100644
> --- a/locale/C-messages.c
> +++ b/locale/C-messages.c
> @@ -26,7 +26,7 @@ const struct __locale_data _nl_C_LC_MESSAGES attribute_hidden =
>  {
>    _nl_C_name,
>    NULL, 0, 0,			/* no file mapped */
> -  { NULL, },			/* no cached data */
> +  NULL,				/* No cached data.  */
>    UNDELETABLE,
>    0,
>    5,
> diff --git a/locale/C-monetary.c b/locale/C-monetary.c
> index 295f7a93f0..9c752bc1b6 100644
> --- a/locale/C-monetary.c
> +++ b/locale/C-monetary.c
> @@ -27,7 +27,7 @@ const struct __locale_data _nl_C_LC_MONETARY attribute_hidden =
>  {
>    _nl_C_name,
>    NULL, 0, 0,			/* no file mapped */
> -  { NULL, },			/* no cached data */
> +  NULL,				/* No cached data.  */
>    UNDELETABLE,
>    0,
>    46,
> diff --git a/locale/C-name.c b/locale/C-name.c
> index c9b03ef176..0c04b447cd 100644
> --- a/locale/C-name.c
> +++ b/locale/C-name.c
> @@ -26,7 +26,7 @@ const struct __locale_data _nl_C_LC_NAME attribute_hidden =
>  {
>    _nl_C_name,
>    NULL, 0, 0,			/* no file mapped */
> -  { NULL, },			/* no cached data */
> +  NULL,				/* No cached data.  */
>    UNDELETABLE,
>    0,
>    7,
> diff --git a/locale/C-numeric.c b/locale/C-numeric.c
> index 178f61cd89..6f67deca1f 100644
> --- a/locale/C-numeric.c
> +++ b/locale/C-numeric.c
> @@ -23,7 +23,7 @@ const struct __locale_data _nl_C_LC_NUMERIC attribute_hidden =
>  {
>    _nl_C_name,
>    NULL, 0, 0,			/* no file mapped */
> -  { NULL, },			/* no cached data */
> +  NULL, 			/* No cached data.  */
>    UNDELETABLE,
>    0,
>    6,
> diff --git a/locale/C-paper.c b/locale/C-paper.c
> index 06822385f6..56b9519801 100644
> --- a/locale/C-paper.c
> +++ b/locale/C-paper.c
> @@ -26,7 +26,7 @@ const struct __locale_data _nl_C_LC_PAPER attribute_hidden =
>  {
>    _nl_C_name,
>    NULL, 0, 0,			/* no file mapped */
> -  { NULL, },			/* no cached data */
> +  NULL,				/* No cached data.  */
>    UNDELETABLE,
>    0,
>    3,
> diff --git a/locale/C-telephone.c b/locale/C-telephone.c
> index 7f71f605c5..1e8e8b7a66 100644
> --- a/locale/C-telephone.c
> +++ b/locale/C-telephone.c
> @@ -26,7 +26,7 @@ const struct __locale_data _nl_C_LC_TELEPHONE attribute_hidden =
>  {
>    _nl_C_name,
>    NULL, 0, 0,			/* no file mapped */
> -  { NULL, },			/* no cached data */
> +  NULL,				/* No cached data.  */
>    UNDELETABLE,
>    0,
>    5,
> diff --git a/locale/C-time.c b/locale/C-time.c
> index ebd79591e2..78d87046fb 100644
> --- a/locale/C-time.c
> +++ b/locale/C-time.c
> @@ -26,7 +26,7 @@ const struct __locale_data _nl_C_LC_TIME attribute_hidden =
>  {
>    _nl_C_name,
>    NULL, 0, 0,			/* no file mapped */
> -  { NULL, },			/* no cached data */
> +  NULL, 			/* No cached data.  */
>    UNDELETABLE,
>    0,
>    159,
> diff --git a/locale/localeinfo.h b/locale/localeinfo.h
> index 8ce072b7b4..01ec5535bb 100644
> --- a/locale/localeinfo.h
> +++ b/locale/localeinfo.h
> @@ -59,13 +59,13 @@ struct __locale_data
>    } alloc;
>  
>    /* This provides a slot for category-specific code to cache data
> -     computed about this locale.  This is deallocated at the start of
> -     _nl_unload_locale.  */
> -  union
> -  {
> -    struct lc_time_data *time;
> -    const struct gconv_fcts *ctype;
> -  } private;
> +     computed about this locale.  Type of the data pointed to:
> +
> +     LC_CTYPE   struct gconv_fcts (get_gconv_fcts, __wcsmbs_load_conv)
> +     LC_TIME    struct lc_time_data (_nl_init_alt_digit, _nl_init_era_entries)
> +
> +     This data deallocated at the start of _nl_unload_locale.  */
> +  void *private;
>  
>    unsigned int usage_count;	/* Counter for users.  */
>  
> diff --git a/time/alt_digit.c b/time/alt_digit.c
> index 7ed9b6b0a5..331cb395ce 100644
> --- a/time/alt_digit.c
> +++ b/time/alt_digit.c
> @@ -30,19 +30,18 @@ __libc_rwlock_define (extern, __libc_setlocale_lock attribute_hidden)
>  #define CURRENT_WSTR(item) \
>    ((wchar_t *) current->values[_NL_ITEM_INDEX (item)].wstr)
>  
> -static void
> +static struct lc_time_data *
>  _nl_init_alt_digit (struct __locale_data *current)
>  {
> -  struct lc_time_data *data;
> +  struct lc_time_data *data = current->private;
>  
> -  if (current->private.time == NULL)
> +  if (data == NULL)
>      {
> -      current->private.time = malloc (sizeof *current->private.time);
> -      if (current->private.time == NULL)
> -	return;
> -      memset (current->private.time, 0, sizeof *current->private.time);
> +      data = calloc (sizeof *data, 1);
> +      if (data == NULL)
> +	return NULL;
> +      current->private = data;
>      }
> -  data = current->private.time;
>  
>    if (! data->alt_digits_initialized)
>      {
> @@ -65,6 +64,7 @@ _nl_init_alt_digit (struct __locale_data *current)
>  	}
>      }
>  
> +  return data;
>  }
>  
>  const char *
> @@ -77,13 +77,11 @@ _nl_get_alt_digit (unsigned int number, struct __locale_data *current)
>  
>    __libc_rwlock_wrlock (__libc_setlocale_lock);
>  
> -  if (current->private.time == NULL
> -      || ! current->private.time->alt_digits_initialized)
> -    _nl_init_alt_digit (current);
> +  struct lc_time_data *data = _nl_init_alt_digit (current);
>  
> -  result = ((current->private.time != NULL
> -	     && current->private.time->alt_digits != NULL)
> -	    ? current->private.time->alt_digits[number]
> +  result = ((data != NULL
> +	     && data->alt_digits != NULL)
> +	    ? data->alt_digits[number]
>  	    : NULL);
>  
>    __libc_rwlock_unlock (__libc_setlocale_lock);
> @@ -96,21 +94,20 @@ const wchar_t *
>  _nl_get_walt_digit (unsigned int number, struct __locale_data *current)
>  {
>    const wchar_t *result = NULL;
> -  struct lc_time_data *data;
>  
>    if (number >= 100 || CURRENT_WSTR (_NL_WALT_DIGITS)[0] == L'\0')
>      return NULL;
>  
>    __libc_rwlock_wrlock (__libc_setlocale_lock);
>  
> -  if (current->private.time == NULL)
> +  struct lc_time_data *data = current->private;
> +  if (data == NULL)
>      {
> -      current->private.time = malloc (sizeof *current->private.time);
> -      if (current->private.time == NULL)
> +      data = calloc (sizeof *data, 1);
> +      if (data == NULL)
>  	goto out;
> -      memset (current->private.time, 0, sizeof *current->private.time);
> +      current->private = data;
>      }
> -  data = current->private.time;
>  
>    if (! data->walt_digits_initialized)
>      {
> @@ -156,12 +153,8 @@ _nl_parse_alt_digit (const char **strp, struct __locale_data *current)
>  
>    __libc_rwlock_wrlock (__libc_setlocale_lock);
>  
> -  if (current->private.time == NULL
> -      || ! current->private.time->alt_digits_initialized)
> -    _nl_init_alt_digit (current);
> -
> -  if (current->private.time != NULL
> -      && current->private.time->alt_digits != NULL)
> +  struct lc_time_data *data = _nl_init_alt_digit (current);
> +  if (data != NULL && data->alt_digits != NULL)
>      /* Matching is not unambiguous.  The alternative digits could be like
>         I, II, III, ... and the first one is a substring of the second
>         and third.  Therefore we must keep on searching until we found
> @@ -169,7 +162,7 @@ _nl_parse_alt_digit (const char **strp, struct __locale_data *current)
>         the standard.  */
>      for (cnt = 0; cnt < 100; ++cnt)
>        {
> -	const char *const dig = current->private.time->alt_digits[cnt];
> +	const char *const dig = data->alt_digits[cnt];
>  	size_t len = strlen (dig);
>  
>  	if (len > maxlen && strncmp (dig, str, len) == 0)
> diff --git a/time/era.c b/time/era.c
> index d4b538c7b0..7f18071888 100644
> --- a/time/era.c
> +++ b/time/era.c
> @@ -35,7 +35,7 @@ __libc_rwlock_define (extern, __libc_setlocale_lock attribute_hidden)
>  
>  /* Look up the era information in CURRENT's locale strings and
>     cache it in CURRENT->private.  */
> -static void
> +static struct lc_time_data *
>  _nl_init_era_entries (struct __locale_data *current)
>  {
>    size_t cnt;
> @@ -43,18 +43,22 @@ _nl_init_era_entries (struct __locale_data *current)
>  
>    /* Avoid touching CURRENT if there is no data at all, for _nl_C_LC_TIME.  */
>    if (CURRENT_WORD (_NL_TIME_ERA_NUM_ENTRIES) == 0)
> -    return;
> +    return NULL;
> +
> +  data = current->private;
> +  if (data != NULL && atomic_load_acquire (&data->era_initialized))
> +    return data;
>  
>    __libc_rwlock_wrlock (__libc_setlocale_lock);
>  
> -  if (current->private.time == NULL)
> +  data = current->private;
> +  if (data == NULL)
>      {
> -      current->private.time = malloc (sizeof *current->private.time);
> -      if (current->private.time == NULL)
> +      data = calloc (sizeof *data, 1);
> +      if (data == NULL)
>  	goto out;
> -      memset (current->private.time, 0, sizeof *current->private.time);
> +      current->private = data;
>      }
> -  data = current->private.time;
>  
>    if (! data->era_initialized)
>      {
> @@ -130,33 +134,30 @@ _nl_init_era_entries (struct __locale_data *current)
>  	    }
>  	}
>  
> -      data->era_initialized = 1;
> +      atomic_store_release (&data->era_initialized, 1);
>      }
>  
>   out:
>    __libc_rwlock_unlock (__libc_setlocale_lock);
> +  return data;
>  }
>  
>  struct era_entry *
>  _nl_get_era_entry (const struct tm *tp, struct __locale_data *current)
>  {
> -  if (current->private.time == NULL || !current->private.time->era_initialized)
> -    _nl_init_era_entries (current);
> +  struct lc_time_data *data = _nl_init_era_entries (current);
>  
> -  if (current->private.time != NULL)
> +  if (data != NULL)
>      {
>        /* Now compare date with the available eras.  */
>        const int32_t tdate[3] = { tp->tm_year, tp->tm_mon, tp->tm_mday };
>        size_t cnt;
> -      for (cnt = 0; cnt < current->private.time->num_eras; ++cnt)
> -	if ((ERA_DATE_CMP (current->private.time->eras[cnt].start_date, tdate)
> -	     && ERA_DATE_CMP (tdate,
> -			      current->private.time->eras[cnt].stop_date))
> -	    || (ERA_DATE_CMP (current->private.time->eras[cnt].stop_date,
> -			      tdate)
> -		&& ERA_DATE_CMP (tdate,
> -				 current->private.time->eras[cnt].start_date)))
> -	  return &current->private.time->eras[cnt];
> +      for (cnt = 0; cnt < data->num_eras; ++cnt)
> +	if ((ERA_DATE_CMP (data->eras[cnt].start_date, tdate)
> +	     && ERA_DATE_CMP (tdate, data->eras[cnt].stop_date))
> +	    || (ERA_DATE_CMP (data->eras[cnt].stop_date, tdate)
> +		&& ERA_DATE_CMP (tdate, data->eras[cnt].start_date)))
> +	  return &data->eras[cnt];
>      }
>  
>    return NULL;
> @@ -166,9 +167,6 @@ _nl_get_era_entry (const struct tm *tp, struct __locale_data *current)
>  struct era_entry *
>  _nl_select_era_entry (int cnt, struct __locale_data *current)
>  {
> -  if (current->private.time == NULL || !current->private.time->era_initialized)
> -    _nl_init_era_entries (current);
> -
> -  return (current->private.time == NULL
> -	  ? NULL : &current->private.time->eras[cnt]);
> +  struct lc_time_data *data = _nl_init_era_entries (current);
> +  return data == NULL ? NULL : &data->eras[cnt];
>  }
> diff --git a/time/lc-time-cleanup.c b/time/lc-time-cleanup.c
> index f844e04905..bcf6d2fbc9 100644
> --- a/time/lc-time-cleanup.c
> +++ b/time/lc-time-cleanup.c
> @@ -22,10 +22,10 @@
>  void
>  _nl_cleanup_time (struct __locale_data *locale)
>  {
> -  struct lc_time_data *const data = locale->private.time;
> +  struct lc_time_data *const data = locale->private;
>    if (data != NULL)
>      {
> -      locale->private.time = NULL;
> +      locale->private = NULL;
>  
>        free (data->eras);
>        free (data->alt_digits);
> diff --git a/wcsmbs/wcsmbsload.c b/wcsmbs/wcsmbsload.c
> index af539a099a..2650834e29 100644
> --- a/wcsmbs/wcsmbsload.c
> +++ b/wcsmbs/wcsmbsload.c
> @@ -155,7 +155,7 @@ __wcsmbs_load_conv (struct __locale_data *new_category)
>  
>    /* We should repeat the test since while we waited some other thread
>       might have run this function.  */
> -  if (__glibc_likely (new_category->private.ctype == NULL))
> +  if (__glibc_likely (new_category->private == NULL))
>      {
>        /* We must find the real functions.  */
>        const char *charset_name;
> @@ -199,10 +199,10 @@ __wcsmbs_load_conv (struct __locale_data *new_category)
>  	  free (new_fcts);
>  
>  	failed:
> -	  new_category->private.ctype = &__wcsmbs_gconv_fcts_c;
> +	  new_category->private = (void *) &__wcsmbs_gconv_fcts_c;
>  	}
>        else
> -	new_category->private.ctype = new_fcts;
> +	new_category->private = new_fcts;
>      }
>  
>    __libc_rwlock_unlock (__libc_setlocale_lock);
> @@ -263,10 +263,10 @@ __wcsmbs_named_conv (struct gconv_fcts *copy, const char *name)
>  void
>  _nl_cleanup_ctype (struct __locale_data *locale)
>  {
> -  const struct gconv_fcts *const data = locale->private.ctype;
> +  const struct gconv_fcts *const data = locale->private;
>    if (data != NULL && data != &__wcsmbs_gconv_fcts_c)
>      {
> -      locale->private.ctype = NULL;
> +      locale->private = NULL;
>  
>        /* Free the old conversions.  */
>        __gconv_close_transform (data->tomb, data->tomb_nsteps);
> diff --git a/wcsmbs/wcsmbsload.h b/wcsmbs/wcsmbsload.h
> index 1ff51e0f8a..8bbd34ba02 100644
> --- a/wcsmbs/wcsmbsload.h
> +++ b/wcsmbs/wcsmbsload.h
> @@ -66,13 +66,15 @@ extern const struct __locale_data _nl_C_LC_CTYPE attribute_hidden;
>  static inline const struct gconv_fcts *
>  get_gconv_fcts (struct __locale_data *data)
>  {
> -  if (__glibc_unlikely (data->private.ctype == NULL))
> +  struct gconv_fcts *private = data->private;
> +  if (private == NULL)
>      {
> -      if (__glibc_unlikely (data == &_nl_C_LC_CTYPE))
> +      if (data == &_nl_C_LC_CTYPE)
>  	return &__wcsmbs_gconv_fcts_c;
>        __wcsmbs_load_conv (data);
> +      private = data->private;
>      }
> -  return data->private.ctype;
> +  return private;
>  }
>  
>  #endif	/* wcsmbsload.h */