Re: [PATCH 15/26] locale: Add more cached data to LC_CTYPE

[Florian Weimer <fweimer@redhat.com>]

* Adhemerval Zanella:

> On 17/03/2022 16:30, Florian Weimer via Libc-alpha wrote:
>> This data will be used in number formatting.
>
> Looks ok, some minor comments below.
>
> Reviewed-by: Adhemerval Zanella  <adhemerval.zanella@linaro.org>

>> diff --git a/locale/loadlocale.c b/locale/loadlocale.c
>> index 9069bafcd8..ce78dfd071 100644
>> --- a/locale/loadlocale.c
>> +++ b/locale/loadlocale.c
>> @@ -31,7 +31,6 @@
>>  #include <not-cancel.h>
>>  #include "localeinfo.h"
>>  
>> -
>
> Spurious line removal.

Fixed.

>>  static const size_t _nl_category_num_items[] =
>>  {
>>  #define DEFINE_CATEGORY(category, category_name, items, a) \
>> @@ -62,6 +61,61 @@ static const enum value_type *const _nl_value_types[] =
>>  #undef DEFINE_CATEGORY
>>  };
>>  
>> +/* Fill in LOCDATA->private for the LC_CTYPE category.  */
>> +static void
>> +_nl_intern_locale_data_fill_cache_ctype (struct __locale_data *locdata)
>> +{
>> +  struct lc_ctype_data *data = locdata->private;
>> +
>> +  /* Default to no translation.  Assumes zero initialization of *data.  */
>> +  memset (data->outdigit_bytes, 1, 10);
>
> Use sizeof data->outdigit_bytes here.

Also fixed.

>> +/* Ancillary data for LC_CTYPE.  Co-allocated after struct
>> +   __locale_data by _nl_intern_locale_data.  */
>> +struct lc_ctype_data
>> +{
>> +  /* See get_gconv_fcts and __wcsmbs_load_conv.  */
>> +  const struct gconv_fcts *fcts;
>> +
>> +  /* If false, outdigit just maps to the ASCII digits.  */
>> +  bool outdigit_translation_needed;
>> +
>> +  /* Cached multi-byte string lengths.  This could be added to the
>> +     locale data itself if the format is changed (which impacts
>> +     existing statically linked binaries).  */
>> +
>> +  /* For the outdigit decimal digits (copied from LC_CTYPE).  */
>> +  unsigned char outdigit_bytes[10];
>> +
>> +  /* If all outdigit_bytes elements are equal, this is that value,
>> +     otherwise it is 0.  */
>> +  unsigned char outdigit_bytes_all_equal;
>
> Why not _Bool?

And use outdigit_bytes[0] instead of this field?  The use looks like
this:

  if (ctype->outdigit_bytes_all_equal > 0)
    return (last - first) * ctype->outdigit_bytes_all_equal;

I thought this was simple enough.

Thanks,
Florian