* [PATCH v2 0/2] Improve LC_MONETARY handling.
@ 2022-02-24 21:45 Carlos O'Donell
2022-02-24 21:45 ` [PATCH v2 1/2] localedef: Update LC_MONETARY handling (Bug 28845) Carlos O'Donell
2022-02-24 21:45 ` [PATCH v2 2/2] localedata: Do not generate output if warnings were present Carlos O'Donell
0 siblings, 2 replies; 5+ messages in thread
From: Carlos O'Donell @ 2022-02-24 21:45 UTC (permalink / raw)
To: libc-alpha, dj
In glibc 2.35 we released C.UTF-8, but if you try to compile
the locale without '-c' you get the following error:
localedef -i C -f UTF-8 C.UTF-8
[error] LC_MONETARY: value for field `mon_decimal_point'
must not be an empty string
[error] no output file produced because errors were issued
The first patch in the series fixes bug 28845 and cleans up
LC_MONETARY handling in ld-monetary.c (monetary_finish)
for default values and generated errors. In general we
downgrade the errors and make them warnings.
Lastly, to prevent this from happening again we remove the
'-c' from glibc localedef uses since we do not want to force
output generation and we should always have locales that
compile cleanly.
Where locales are not clean we should implement specific
warning disabling e.g. --no-warnings=ascii for SHIFT_JIS
and SHIFT_JISX0213 (non-ascii locales). Any future locale
warnings should fail during locale compilation (install
or testing).
Changes from v1:
- Fix spelling mistakes in ld-monetary.c
- Expand comments for mon_grouping to explain "\177" oddity.
- Keep @flags="" in Makefile for future debugging uses.
Carlos O'Donell (2):
localedef: Update LC_MONETARY handling (Bug 28845)
localedata: Do not generate output if warnings were present.
locale/programs/ld-monetary.c | 182 +++++++++++++++++++++++++++-------
localedata/Makefile | 3 +-
localedata/gen-locale.sh | 10 +-
3 files changed, 155 insertions(+), 40 deletions(-)
--
2.34.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2 1/2] localedef: Update LC_MONETARY handling (Bug 28845)
2022-02-24 21:45 [PATCH v2 0/2] Improve LC_MONETARY handling Carlos O'Donell
@ 2022-02-24 21:45 ` Carlos O'Donell
2022-02-25 5:54 ` DJ Delorie
2022-02-24 21:45 ` [PATCH v2 2/2] localedata: Do not generate output if warnings were present Carlos O'Donell
1 sibling, 1 reply; 5+ messages in thread
From: Carlos O'Donell @ 2022-02-24 21:45 UTC (permalink / raw)
To: libc-alpha, dj
ISO C17, POSIX Issue 7, and ISO 30112 all allow the char*
types to be empty strings i.e. "", integer or char values to
be -1 or CHAR_MAX respectively, with the exception of
decimal_point which must be non-empty in ISO C. Note that
the defaults for mon_grouping vary, but are functionaly
equivalent e.g. "\177" (no further grouping reuqired) vs.
"" (no grouping defined for all groups).
We include a broad comment talking about harmonizing ISO C,
POSIX, ISO 30112, and the default C/POSIX locale for glibc.
We reorder all setting based on locale/categories.def order.
We soften all missing definitions from errors to warnings when
defaults exist.
Given that ISO C, POSIX and ISO 30112 allow the empty string
we change LC_MONETARY handling of mon_decimal_point to allow
the empty string. If mon_decimal_point is not defined at all
then we pick the existing legacy glibc default value of
<U002E> i.e. ".".
We also set the default for mon_thousands_sep_wc at the
same time as mon_thousands_sep, but this is not a change in
behaviour, it is always either a matching value or L'\0',
but if in the future we change the default to a non-empty
string we would need to update both at the same time.
Tested on x86_64 and i686 without regressions.
Tested with install-locale-archive target.
Tested with install-locale-files target.
---
locale/programs/ld-monetary.c | 182 +++++++++++++++++++++++++++-------
1 file changed, 146 insertions(+), 36 deletions(-)
diff --git a/locale/programs/ld-monetary.c b/locale/programs/ld-monetary.c
index 3b0412b405..e78f40ffb4 100644
--- a/locale/programs/ld-monetary.c
+++ b/locale/programs/ld-monetary.c
@@ -196,21 +196,105 @@ No definition for %s category found"), "LC_MONETARY");
}
}
+ /* Generally speaking there are 3 standards the define the default,
+ warning, and error behaviour of LC_MONETARY. They are ISO/IEC TR 30112,
+ ISO/IEC 9899:2018 (ISO C17), and POSIX.1-2017. Within 30112 we have the
+ definition of a standard i18n FDCC-set, which for LC_MONETARY has the
+ following default values:
+ int_curr_symbol ""
+ currency_symbol ""
+ mon_decimal_point "<U002C>" i.e. ","
+ mon_thousand_sep ""
+ mon_grouping "\177" i.e. terminating -1
+ positive_sign ""
+ negative_sign "<U002E>" i.e. "."
+ int_frac_digits -1
+ frac_digits -1
+ p_cs_precedes -1
+ p_sep_by_space -1
+ n_cs_precedes -1
+ n_sep_by_space -1
+ p_sign_posn -1
+ n_sign_posn -1
+ Under 30112 a keyword that is not provided implies an empty string ""
+ for string values or a -1 for integer values, and indicates the value
+ is unspecified with no default implied. No errors are considered.
+ The exception is mon_grouping which is a string with a terminating
+ CHAR_MAX.
+ For POSIX Issue 7 we have:
+ https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap07.html
+ and again values not provided default to "" or -1, and indicate the value
+ is not available to the locale. The exception is mon_grouping which is
+ a string with a terminating CHAR_MAX. For the POSIX locale the values of
+ LC_MONETARY should be:
+ int_curr_symbol ""
+ currency_symbol ""
+ mon_decimal_point ""
+ mon_thousands_sep ""
+ mon_grouping "\177" i.e. terminating -1
+ positive_sign ""
+ negative_sign ""
+ int_frac_digits -1
+ frac_digits -1
+ p_cs_precedes -1
+ p_sep_by_space -1
+ n_cs_precedes -1
+ n_sep_by_space -1
+ p_sign_posn -1
+ n_sign_posn -1
+ int_p_cs_precedes -1
+ int_p_sep_by_space -1
+ int_n_cs_precedes -1
+ int_n_sep_by_space -1
+ int_p_sign_posn -1
+ int_n_sign_posn -1
+ Like with 30112, POSIX also considers no error if the keywords are
+ missing, only that if the cateory as a whole is missing the referencing
+ of the category results in unspecified behaviour.
+ For ISO C17 there is no default value provided, but the localeconv
+ specification in 7.11.2.1 admits that members of char * type may point
+ to "" to indicate a value is not available or is of length zero.
+ The exception is decimal_point (not mon_decimal_point) which must be a
+ defined non-empty string. The values of char, which are generally
+ mapped to integer values in 30112 and POSIX, must be non-negative
+ numbers that map to CHAR_MAX when a value is not available in the
+ locale.
+ In ISO C17 for the "C" locale all values are empty strings "", or
+ CHAR_MAX, with the exception of decimal_point which is "." (defined
+ in LC_NUMERIC). ISO C17 makes no exception for mon_grouping like
+ 30112 and POSIX, but a value of "" is functionally equivalent to
+ "\177" since neither defines a grouping (though the latter terminates
+ the grouping).
+
+ Lastly, we must consider the legacy C/POSIX locale that implemented
+ as a builtin in glibc and wether a default value mapping to the
+ C/POSIX locale may benefit the user from a compatibility perspective.
+
+ Thus given 30112, POSIX, ISO C, and the builtin C/POSIX locale we
+ need to pick appropriate defaults below. */
+
+ /* The members of LC_MONETARY are handled in the order of their definition
+ in locale/categories.def. Please keep them in that order. */
+
+ /* The purpose of TEST_ELEM is to define a default value for the fields
+ in the category if the field was not defined in the cateory. If the
+ category was present but we didn't see a definition for the field then
+ we also issue a warning, otherwise the only warning you get is the one
+ earlier when a default category is created (completely missing category).
+ This missing field warning is glibc-specific since no standard requires
+ this warning, but we consider it valuable to print a warning for all
+ missing fields in the category. */
#define TEST_ELEM(cat, initval) \
if (monetary->cat == NULL) \
{ \
if (! nothing) \
- record_error (0, 0, _("%s: field `%s' not defined"), \
- "LC_MONETARY", #cat); \
+ record_warning (_("%s: field `%s' not defined"), \
+ "LC_MONETARY", #cat); \
monetary->cat = initval; \
}
+ /* Keyword: int_curr_symbol. */
TEST_ELEM (int_curr_symbol, "");
- TEST_ELEM (currency_symbol, "");
- TEST_ELEM (mon_thousands_sep, "");
- TEST_ELEM (positive_sign, "");
- TEST_ELEM (negative_sign, "");
-
/* The international currency symbol must come from ISO 4217. */
if (monetary->int_curr_symbol != NULL)
{
@@ -247,41 +331,63 @@ not correspond to a valid name in ISO 4217 [--no-warnings=intcurrsym]"),
}
}
- /* The decimal point must not be empty. This is not said explicitly
- in POSIX but ANSI C (ISO/IEC 9899) says in 4.4.2.1 it has to be
- != "". */
+ /* Keyword: currency_symbol */
+ TEST_ELEM (currency_symbol, "");
+
+ /* Keyword: mon_decimal_point */
+ /* ISO C17 7.11.2.1.3 explicitly allows mon_decimal_point to be the
+ empty string e.g. "". This indicates the value is not available in the
+ current locale or is of zero length. However, if the value was never
+ defined then we issue a warning and use a glibc-specific default. ISO
+ 30112 in the i18n FDCC-Set uses <U002C> ",", and POSIX Issue 7 in the
+ POSIX locale uses "". It is specific to glibc that the default is <U002E>
+ "."; we retain this existing behaviour for backwards compatibility. */
if (monetary->mon_decimal_point == NULL)
{
if (! nothing)
- record_error (0, 0, _("%s: field `%s' not defined"),
- "LC_MONETARY", "mon_decimal_point");
+ record_warning (_("%s: field `%s' not defined, using defaults"),
+ "LC_MONETARY", "mon_decimal_point");
monetary->mon_decimal_point = ".";
monetary->mon_decimal_point_wc = L'.';
}
- else if (monetary->mon_decimal_point[0] == '\0' && ! be_quiet && ! nothing)
+
+ /* Keyword: mon_thousands_sep */
+ if (monetary->mon_thousands_sep == NULL)
{
- record_error (0, 0, _("\
-%s: value for field `%s' must not be an empty string"),
- "LC_MONETARY", "mon_decimal_point");
+ if (! nothing)
+ record_warning (_("%s: field `%s' not defined, using defaults"),
+ "LC_MONETARY", "mon_thousands_sep");
+ monetary->mon_thousands_sep = "";
+ monetary->mon_thousands_sep_wc = L'\0';
}
+ /* Keyword: mon_grouping */
if (monetary->mon_grouping_len == 0)
{
if (! nothing)
- record_error (0, 0, _("%s: field `%s' not defined"),
- "LC_MONETARY", "mon_grouping");
-
+ record_warning (_("%s: field `%s' not defined"),
+ "LC_MONETARY", "mon_grouping");
+ /* Missing entries are given 1 element in their bytearray with
+ a value of CHAR_MAX which indicates that "No further grouping
+ is to be performed" (functionally equivalent to ISO C's "C"
+ locale default of ""). */
monetary->mon_grouping = (char *) "\177";
monetary->mon_grouping_len = 1;
}
+ /* Keyword: positive_sign */
+ TEST_ELEM (positive_sign, "");
+
+ /* Keyword: negative_sign */
+ TEST_ELEM (negative_sign, "");
+
#undef TEST_ELEM
#define TEST_ELEM(cat, min, max, initval) \
if (monetary->cat == -2) \
{ \
if (! nothing) \
- record_error (0, 0, _("%s: field `%s' not defined"), \
- "LC_MONETARY", #cat); \
+ record_warning (_("%s: field `%s' not defined"), \
+ "LC_MONETARY", #cat); \
monetary->cat = initval; \
} \
else if ((monetary->cat < min || monetary->cat > max) \
@@ -300,16 +406,11 @@ not correspond to a valid name in ISO 4217 [--no-warnings=intcurrsym]"),
TEST_ELEM (p_sign_posn, -1, 4, -1);
TEST_ELEM (n_sign_posn, -1, 4, -1);
- /* The non-POSIX.2 extensions are optional. */
- if (monetary->duo_int_curr_symbol == NULL)
- monetary->duo_int_curr_symbol = monetary->int_curr_symbol;
- if (monetary->duo_currency_symbol == NULL)
- monetary->duo_currency_symbol = monetary->currency_symbol;
-
- if (monetary->duo_int_frac_digits == -2)
- monetary->duo_int_frac_digits = monetary->int_frac_digits;
- if (monetary->duo_frac_digits == -2)
- monetary->duo_frac_digits = monetary->frac_digits;
+ /* Keyword: crncystr */
+ monetary->crncystr = (char *) xmalloc (strlen (monetary->currency_symbol)
+ + 2);
+ monetary->crncystr[0] = monetary->p_cs_precedes ? '-' : '+';
+ strcpy (&monetary->crncystr[1], monetary->currency_symbol);
#undef TEST_ELEM
#define TEST_ELEM(cat, alt, min, max) \
@@ -327,6 +428,17 @@ not correspond to a valid name in ISO 4217 [--no-warnings=intcurrsym]"),
TEST_ELEM (int_p_sign_posn, p_sign_posn, -1, 4);
TEST_ELEM (int_n_sign_posn, n_sign_posn, -1, 4);
+ /* The non-POSIX.2 extensions are optional. */
+ if (monetary->duo_int_curr_symbol == NULL)
+ monetary->duo_int_curr_symbol = monetary->int_curr_symbol;
+ if (monetary->duo_currency_symbol == NULL)
+ monetary->duo_currency_symbol = monetary->currency_symbol;
+
+ if (monetary->duo_int_frac_digits == -2)
+ monetary->duo_int_frac_digits = monetary->int_frac_digits;
+ if (monetary->duo_frac_digits == -2)
+ monetary->duo_frac_digits = monetary->frac_digits;
+
TEST_ELEM (duo_p_cs_precedes, p_cs_precedes, -1, 1);
TEST_ELEM (duo_p_sep_by_space, p_sep_by_space, -1, 2);
TEST_ELEM (duo_n_cs_precedes, n_cs_precedes, -1, 1);
@@ -349,17 +461,15 @@ not correspond to a valid name in ISO 4217 [--no-warnings=intcurrsym]"),
if (monetary->duo_valid_to == 0)
monetary->duo_valid_to = 99991231;
+ /* Keyword: conversion_rate */
if (monetary->conversion_rate[0] == 0)
{
monetary->conversion_rate[0] = 1;
monetary->conversion_rate[1] = 1;
}
- /* Create the crncystr entry. */
- monetary->crncystr = (char *) xmalloc (strlen (monetary->currency_symbol)
- + 2);
- monetary->crncystr[0] = monetary->p_cs_precedes ? '-' : '+';
- strcpy (&monetary->crncystr[1], monetary->currency_symbol);
+ /* A value for monetary-decimal-point-wc was set when
+ monetary_decimal_point was set, likewise for monetary-thousands-sep-wc. */
}
--
2.34.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2 2/2] localedata: Do not generate output if warnings were present.
2022-02-24 21:45 [PATCH v2 0/2] Improve LC_MONETARY handling Carlos O'Donell
2022-02-24 21:45 ` [PATCH v2 1/2] localedef: Update LC_MONETARY handling (Bug 28845) Carlos O'Donell
@ 2022-02-24 21:45 ` Carlos O'Donell
1 sibling, 0 replies; 5+ messages in thread
From: Carlos O'Donell @ 2022-02-24 21:45 UTC (permalink / raw)
To: libc-alpha, dj
With LC_MONETARY parsing fixed we can now generate locales
without forcing output with '-c'.
Removing '-c' from localedef invocation is the equivalent of
using -Werror for localedef. The glibc locale sources should
always be clean and free from warnings.
We remove '-c' from both test locale generation and the targets
used for installing locales e.g. install-locale-archive, and
install-locale-files.
Tested on x86_64 and i686 without regressions.
Tested with install-locale-archive target.
Tested with install-locale-files target.
Reviewed-by: DJ Delorie <dj@redhat.com>
---
localedata/Makefile | 4 ++--
localedata/gen-locale.sh | 10 ++++++++--
2 files changed, 10 insertions(+), 4 deletions(-)
diff --git a/localedata/Makefile b/localedata/Makefile
index 9ae2e5c161..7741ac3b5e 100644
--- a/localedata/Makefile
+++ b/localedata/Makefile
@@ -468,11 +468,11 @@ define build-one-locale
endef
$(INSTALL-SUPPORTED-LOCALE-ARCHIVE): install-locales-dir
- @flags="-c"; \
+ @flags=""; \
$(build-one-locale)
$(INSTALL-SUPPORTED-LOCALE-FILES): install-locales-dir
- @flags="-c --no-archive --no-hard-links"; \
+ @flags="--no-archive --no-hard-links"; \
$(build-one-locale)
tst-setlocale-ENV = LC_ALL=ja_JP.EUC-JP
diff --git a/localedata/gen-locale.sh b/localedata/gen-locale.sh
index 7fce35f212..8053c816a6 100644
--- a/localedata/gen-locale.sh
+++ b/localedata/gen-locale.sh
@@ -54,8 +54,14 @@ modifier=`echo $locfile|sed 's|[^.]*[.]\([^@ ]*\)\(@[^ ]*\)\?/LC_CTYPE|\2|'`
echo "Generating locale $locale.$charmap: this might take a while..."
-# Run quietly and force output.
-flags="--quiet -c"
+# Do not force output with '-c', all locales should compile without
+# warning or errors. There is likewise no need to run quietly with
+# '--quiet' since all locales should compile without additional
+# diagnostics. If there are messages printed then we want to see
+# them, fix them, and the associated error or warning. During
+# development it may be beneficialy to put '--quiet -c' here to allow
+# you to develop in-progress locales.
+flags=""
# For SJIS the charmap is SHIFT_JIS. We just want the locale to have
# a slightly nicer name instead of using "*.SHIFT_SJIS", but that
--
2.34.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2 1/2] localedef: Update LC_MONETARY handling (Bug 28845)
2022-02-24 21:45 ` [PATCH v2 1/2] localedef: Update LC_MONETARY handling (Bug 28845) Carlos O'Donell
@ 2022-02-25 5:54 ` DJ Delorie
2022-02-25 15:27 ` Carlos O'Donell
0 siblings, 1 reply; 5+ messages in thread
From: DJ Delorie @ 2022-02-25 5:54 UTC (permalink / raw)
To: Carlos O'Donell; +Cc: libc-alpha, carlos
Two comment changes but otherwise OK.
Reviewed-by: DJ Delorie <dj@redhat.com>
"Carlos O'Donell" <carlos@redhat.com> writes:
> diff --git a/locale/programs/ld-monetary.c b/locale/programs/ld-monetary.c
> + mon_grouping "\177" i.e. terminating -1
\177 is char *max*, not -1. \377 is -1.
> + mon_grouping "\177" i.e. terminating -1
Likewise.
> + /* The purpose of TEST_ELEM is to define a default value for the fields
> + in the category if the field was not defined in the cateory. If the
> + category was present but we didn't see a definition for the field then
> + we also issue a warning, otherwise the only warning you get is the one
> + earlier when a default category is created (completely missing category).
> + This missing field warning is glibc-specific since no standard requires
> + this warning, but we consider it valuable to print a warning for all
> + missing fields in the category. */
Ok.
> #define TEST_ELEM(cat, initval) \
> if (monetary->cat == NULL) \
> { \
> if (! nothing) \
> - record_error (0, 0, _("%s: field `%s' not defined"), \
> - "LC_MONETARY", #cat); \
> + record_warning (_("%s: field `%s' not defined"), \
> + "LC_MONETARY", #cat); \
Ok.
> + /* Keyword: int_curr_symbol. */
> TEST_ELEM (int_curr_symbol, "");
> - TEST_ELEM (currency_symbol, "");
> - TEST_ELEM (mon_thousands_sep, "");
> - TEST_ELEM (positive_sign, "");
> - TEST_ELEM (negative_sign, "");
Ok.
> @@ -247,41 +331,63 @@ not correspond to a valid name in ISO 4217 [--no-warnings=intcurrsym]"),
> }
> }
>
> - /* The decimal point must not be empty. This is not said explicitly
> - in POSIX but ANSI C (ISO/IEC 9899) says in 4.4.2.1 it has to be
> - != "". */
> + /* Keyword: currency_symbol */
> + TEST_ELEM (currency_symbol, "");
> +
> + /* Keyword: mon_decimal_point */
> + /* ISO C17 7.11.2.1.3 explicitly allows mon_decimal_point to be the
> + empty string e.g. "". This indicates the value is not available in the
> + current locale or is of zero length. However, if the value was never
> + defined then we issue a warning and use a glibc-specific default. ISO
> + 30112 in the i18n FDCC-Set uses <U002C> ",", and POSIX Issue 7 in the
> + POSIX locale uses "". It is specific to glibc that the default is <U002E>
> + "."; we retain this existing behaviour for backwards compatibility. */
Ok.
> if (monetary->mon_decimal_point == NULL)
> {
> if (! nothing)
> - record_error (0, 0, _("%s: field `%s' not defined"),
> - "LC_MONETARY", "mon_decimal_point");
> + record_warning (_("%s: field `%s' not defined, using defaults"),
> + "LC_MONETARY", "mon_decimal_point");
Ok.
> - else if (monetary->mon_decimal_point[0] == '\0' && ! be_quiet && ! nothing)
> - record_error (0, 0, _("\
> -%s: value for field `%s' must not be an empty string"),
> - "LC_MONETARY", "mon_decimal_point");
Ok.
> +
> + /* Keyword: mon_thousands_sep */
> + if (monetary->mon_thousands_sep == NULL)
> {
> + if (! nothing)
> + record_warning (_("%s: field `%s' not defined, using defaults"),
> + "LC_MONETARY", "mon_thousands_sep");
> + monetary->mon_thousands_sep = "";
> + monetary->mon_thousands_sep_wc = L'\0';
> }
Ok.
> + /* Keyword: mon_grouping */
> if (monetary->mon_grouping_len == 0)
> {
> if (! nothing)
> - record_error (0, 0, _("%s: field `%s' not defined"),
> - "LC_MONETARY", "mon_grouping");
> -
> + record_warning (_("%s: field `%s' not defined"),
> + "LC_MONETARY", "mon_grouping");
Ok.
> + /* Missing entries are given 1 element in their bytearray with
> + a value of CHAR_MAX which indicates that "No further grouping
> + is to be performed" (functionally equivalent to ISO C's "C"
> + locale default of ""). */
> monetary->mon_grouping = (char *) "\177";
> monetary->mon_grouping_len = 1;
Ok.
> + /* Keyword: positive_sign */
> + TEST_ELEM (positive_sign, "");
> +
> + /* Keyword: negative_sign */
> + TEST_ELEM (negative_sign, "");
> +
Ok.
> #undef TEST_ELEM
> #define TEST_ELEM(cat, min, max, initval) \
> if (monetary->cat == -2) \
> { \
> if (! nothing) \
> - record_error (0, 0, _("%s: field `%s' not defined"), \
> - "LC_MONETARY", #cat); \
> + record_warning (_("%s: field `%s' not defined"), \
> + "LC_MONETARY", #cat); \
Ok.
> @@ -300,16 +406,11 @@ not correspond to a valid name in ISO 4217 [--no-warnings=intcurrsym]"),
> TEST_ELEM (p_sign_posn, -1, 4, -1);
> TEST_ELEM (n_sign_posn, -1, 4, -1);
>
> - /* The non-POSIX.2 extensions are optional. */
> - if (monetary->duo_int_curr_symbol == NULL)
> - monetary->duo_int_curr_symbol = monetary->int_curr_symbol;
> - if (monetary->duo_currency_symbol == NULL)
> - monetary->duo_currency_symbol = monetary->currency_symbol;
> -
> - if (monetary->duo_int_frac_digits == -2)
> - monetary->duo_int_frac_digits = monetary->int_frac_digits;
> - if (monetary->duo_frac_digits == -2)
> - monetary->duo_frac_digits = monetary->frac_digits;
Ok.
> + /* Keyword: crncystr */
> + monetary->crncystr = (char *) xmalloc (strlen (monetary->currency_symbol)
> + + 2);
> + monetary->crncystr[0] = monetary->p_cs_precedes ? '-' : '+';
> + strcpy (&monetary->crncystr[1], monetary->currency_symbol);
Ok.
> @@ -327,6 +428,17 @@ not correspond to a valid name in ISO 4217 [--no-warnings=intcurrsym]"),
> TEST_ELEM (int_p_sign_posn, p_sign_posn, -1, 4);
> TEST_ELEM (int_n_sign_posn, n_sign_posn, -1, 4);
>
> + /* The non-POSIX.2 extensions are optional. */
> + if (monetary->duo_int_curr_symbol == NULL)
> + monetary->duo_int_curr_symbol = monetary->int_curr_symbol;
> + if (monetary->duo_currency_symbol == NULL)
> + monetary->duo_currency_symbol = monetary->currency_symbol;
> +
> + if (monetary->duo_int_frac_digits == -2)
> + monetary->duo_int_frac_digits = monetary->int_frac_digits;
> + if (monetary->duo_frac_digits == -2)
> + monetary->duo_frac_digits = monetary->frac_digits;
> +
Ok.
> + /* Keyword: conversion_rate */
Ok.
>
> - /* Create the crncystr entry. */
> - monetary->crncystr = (char *) xmalloc (strlen (monetary->currency_symbol)
> - + 2);
> - monetary->crncystr[0] = monetary->p_cs_precedes ? '-' : '+';
> - strcpy (&monetary->crncystr[1], monetary->currency_symbol);
> + /* A value for monetary-decimal-point-wc was set when
> + monetary_decimal_point was set, likewise for monetary-thousands-sep-wc. */
> }
Ok.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2 1/2] localedef: Update LC_MONETARY handling (Bug 28845)
2022-02-25 5:54 ` DJ Delorie
@ 2022-02-25 15:27 ` Carlos O'Donell
0 siblings, 0 replies; 5+ messages in thread
From: Carlos O'Donell @ 2022-02-25 15:27 UTC (permalink / raw)
To: DJ Delorie; +Cc: libc-alpha
On 2/25/22 00:54, DJ Delorie wrote:
>
> Two comment changes but otherwise OK.
>
> Reviewed-by: DJ Delorie <dj@redhat.com>
Thanks. I pushed a v3 with these fixes.
> "Carlos O'Donell" <carlos@redhat.com> writes:
>> diff --git a/locale/programs/ld-monetary.c b/locale/programs/ld-monetary.c
>
>> + mon_grouping "\177" i.e. terminating -1
>
> \177 is char *max*, not -1. \377 is -1.
>
>> + mon_grouping "\177" i.e. terminating -1
>
> Likewise.
>
>> + /* The purpose of TEST_ELEM is to define a default value for the fields
>> + in the category if the field was not defined in the cateory. If the
>> + category was present but we didn't see a definition for the field then
>> + we also issue a warning, otherwise the only warning you get is the one
>> + earlier when a default category is created (completely missing category).
>> + This missing field warning is glibc-specific since no standard requires
>> + this warning, but we consider it valuable to print a warning for all
>> + missing fields in the category. */
>
> Ok.
>
>> #define TEST_ELEM(cat, initval) \
>> if (monetary->cat == NULL) \
>> { \
>> if (! nothing) \
>> - record_error (0, 0, _("%s: field `%s' not defined"), \
>> - "LC_MONETARY", #cat); \
>> + record_warning (_("%s: field `%s' not defined"), \
>> + "LC_MONETARY", #cat); \
>
> Ok.
>
>> + /* Keyword: int_curr_symbol. */
>> TEST_ELEM (int_curr_symbol, "");
>> - TEST_ELEM (currency_symbol, "");
>> - TEST_ELEM (mon_thousands_sep, "");
>> - TEST_ELEM (positive_sign, "");
>> - TEST_ELEM (negative_sign, "");
>
> Ok.
>
>> @@ -247,41 +331,63 @@ not correspond to a valid name in ISO 4217 [--no-warnings=intcurrsym]"),
>> }
>> }
>>
>> - /* The decimal point must not be empty. This is not said explicitly
>> - in POSIX but ANSI C (ISO/IEC 9899) says in 4.4.2.1 it has to be
>> - != "". */
>> + /* Keyword: currency_symbol */
>> + TEST_ELEM (currency_symbol, "");
>> +
>> + /* Keyword: mon_decimal_point */
>> + /* ISO C17 7.11.2.1.3 explicitly allows mon_decimal_point to be the
>> + empty string e.g. "". This indicates the value is not available in the
>> + current locale or is of zero length. However, if the value was never
>> + defined then we issue a warning and use a glibc-specific default. ISO
>> + 30112 in the i18n FDCC-Set uses <U002C> ",", and POSIX Issue 7 in the
>> + POSIX locale uses "". It is specific to glibc that the default is <U002E>
>> + "."; we retain this existing behaviour for backwards compatibility. */
>
> Ok.
>
>> if (monetary->mon_decimal_point == NULL)
>> {
>> if (! nothing)
>> - record_error (0, 0, _("%s: field `%s' not defined"),
>> - "LC_MONETARY", "mon_decimal_point");
>> + record_warning (_("%s: field `%s' not defined, using defaults"),
>> + "LC_MONETARY", "mon_decimal_point");
>
> Ok.
>
>> - else if (monetary->mon_decimal_point[0] == '\0' && ! be_quiet && ! nothing)
>> - record_error (0, 0, _("\
>> -%s: value for field `%s' must not be an empty string"),
>> - "LC_MONETARY", "mon_decimal_point");
>
> Ok.
>
>> +
>> + /* Keyword: mon_thousands_sep */
>> + if (monetary->mon_thousands_sep == NULL)
>> {
>> + if (! nothing)
>> + record_warning (_("%s: field `%s' not defined, using defaults"),
>> + "LC_MONETARY", "mon_thousands_sep");
>> + monetary->mon_thousands_sep = "";
>> + monetary->mon_thousands_sep_wc = L'\0';
>> }
>
> Ok.
>
>> + /* Keyword: mon_grouping */
>> if (monetary->mon_grouping_len == 0)
>> {
>> if (! nothing)
>> - record_error (0, 0, _("%s: field `%s' not defined"),
>> - "LC_MONETARY", "mon_grouping");
>> -
>> + record_warning (_("%s: field `%s' not defined"),
>> + "LC_MONETARY", "mon_grouping");
>
> Ok.
>
>> + /* Missing entries are given 1 element in their bytearray with
>> + a value of CHAR_MAX which indicates that "No further grouping
>> + is to be performed" (functionally equivalent to ISO C's "C"
>> + locale default of ""). */
>> monetary->mon_grouping = (char *) "\177";
>> monetary->mon_grouping_len = 1;
>
> Ok.
>
>> + /* Keyword: positive_sign */
>> + TEST_ELEM (positive_sign, "");
>> +
>> + /* Keyword: negative_sign */
>> + TEST_ELEM (negative_sign, "");
>> +
>
> Ok.
>
>> #undef TEST_ELEM
>> #define TEST_ELEM(cat, min, max, initval) \
>> if (monetary->cat == -2) \
>> { \
>> if (! nothing) \
>> - record_error (0, 0, _("%s: field `%s' not defined"), \
>> - "LC_MONETARY", #cat); \
>> + record_warning (_("%s: field `%s' not defined"), \
>> + "LC_MONETARY", #cat); \
>
> Ok.
>
>> @@ -300,16 +406,11 @@ not correspond to a valid name in ISO 4217 [--no-warnings=intcurrsym]"),
>> TEST_ELEM (p_sign_posn, -1, 4, -1);
>> TEST_ELEM (n_sign_posn, -1, 4, -1);
>>
>> - /* The non-POSIX.2 extensions are optional. */
>> - if (monetary->duo_int_curr_symbol == NULL)
>> - monetary->duo_int_curr_symbol = monetary->int_curr_symbol;
>> - if (monetary->duo_currency_symbol == NULL)
>> - monetary->duo_currency_symbol = monetary->currency_symbol;
>> -
>> - if (monetary->duo_int_frac_digits == -2)
>> - monetary->duo_int_frac_digits = monetary->int_frac_digits;
>> - if (monetary->duo_frac_digits == -2)
>> - monetary->duo_frac_digits = monetary->frac_digits;
>
> Ok.
>
>> + /* Keyword: crncystr */
>> + monetary->crncystr = (char *) xmalloc (strlen (monetary->currency_symbol)
>> + + 2);
>> + monetary->crncystr[0] = monetary->p_cs_precedes ? '-' : '+';
>> + strcpy (&monetary->crncystr[1], monetary->currency_symbol);
>
> Ok.
>
>> @@ -327,6 +428,17 @@ not correspond to a valid name in ISO 4217 [--no-warnings=intcurrsym]"),
>> TEST_ELEM (int_p_sign_posn, p_sign_posn, -1, 4);
>> TEST_ELEM (int_n_sign_posn, n_sign_posn, -1, 4);
>>
>> + /* The non-POSIX.2 extensions are optional. */
>> + if (monetary->duo_int_curr_symbol == NULL)
>> + monetary->duo_int_curr_symbol = monetary->int_curr_symbol;
>> + if (monetary->duo_currency_symbol == NULL)
>> + monetary->duo_currency_symbol = monetary->currency_symbol;
>> +
>> + if (monetary->duo_int_frac_digits == -2)
>> + monetary->duo_int_frac_digits = monetary->int_frac_digits;
>> + if (monetary->duo_frac_digits == -2)
>> + monetary->duo_frac_digits = monetary->frac_digits;
>> +
>
> Ok.
>
>> + /* Keyword: conversion_rate */
>
> Ok.
>>
>> - /* Create the crncystr entry. */
>> - monetary->crncystr = (char *) xmalloc (strlen (monetary->currency_symbol)
>> - + 2);
>> - monetary->crncystr[0] = monetary->p_cs_precedes ? '-' : '+';
>> - strcpy (&monetary->crncystr[1], monetary->currency_symbol);
>> + /* A value for monetary-decimal-point-wc was set when
>> + monetary_decimal_point was set, likewise for monetary-thousands-sep-wc. */
>> }
>
> Ok.
>
--
Cheers,
Carlos.
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-02-25 15:27 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-02-24 21:45 [PATCH v2 0/2] Improve LC_MONETARY handling Carlos O'Donell
2022-02-24 21:45 ` [PATCH v2 1/2] localedef: Update LC_MONETARY handling (Bug 28845) Carlos O'Donell
2022-02-25 5:54 ` DJ Delorie
2022-02-25 15:27 ` Carlos O'Donell
2022-02-24 21:45 ` [PATCH v2 2/2] localedata: Do not generate output if warnings were present Carlos O'Donell
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).