public inbox for glibc-cvs@sourceware.org
help / color / mirror / Atom feed
From: Carlos O'Donell <carlos@sourceware.org>
To: glibc-cvs@sourceware.org
Subject: [glibc] localedef: Fix handling of empty mon_decimal_point (Bug 28847)
Date: Tue,  1 Feb 2022 17:07:33 +0000 (GMT)	[thread overview]
Message-ID: <20220201170733.49A22385AC1B@sourceware.org> (raw)

https://sourceware.org/git/gitweb.cgi?p=glibc.git;h=1d8e3a2c6636cf0b1b8fa2f869cef6ec10726933

commit 1d8e3a2c6636cf0b1b8fa2f869cef6ec10726933
Author: Carlos O'Donell <carlos@redhat.com>
Date:   Mon Jan 31 00:34:41 2022 -0500

    localedef: Fix handling of empty mon_decimal_point (Bug 28847)
    
    The handling of mon_decimal_point is incorrect when it comes to
    handling the empty "" value.  The existing parser in monetary_read()
    will correctly handle setting the non-wide-character value and the
    wide-character value e.g. STR_ELEM_WC(mon_decimal_point) if they are
    set in the locale definition.  However, in monetary_finish() we have
    conflicting TEST_ELEM() which sets a default value (if the locale
    definition doesn't include one), and subsequent code which looks for
    mon_decimal_point to be NULL to issue a specific error message and set
    the defaults. The latter is unused because TEST_ELEM() always sets a
    default.  The simplest solution is to remove the TEST_ELEM() check,
    and allow the existing check to look to see if mon_decimal_point is
    NULL and set an appropriate default.  The final fix is to move the
    setting of mon_decimal_point_wc so it occurs only when
    mon_decimal_point is being set to a default, keeping both values
    consistent. There is no way to tell the difference between
    mon_decimal_point_wc having been set to the empty string and not
    having been defined at all, for that distinction we must use
    mon_decimal_point being NULL or "", and so we must logically set
    the default together with mon_decimal_point.
    
    Lastly, there are more fixes similar to this that could be made to
    ld-monetary.c, but we avoid that in order to fix just the code
    required for mon_decimal_point, which impacts the ability for C.UTF-8
    to set mon_decimal_point to "", since without this fix we end up with
    an inconsistent setting of mon_decimal_point set to "", but
    mon_decimal_point_wc set to "." which is incorrect.
    
    Tested on x86_64 and i686 without regression.
    Reviewed-by: Florian Weimer <fweimer@redhat.com>

Diff:
---
 locale/programs/ld-monetary.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/locale/programs/ld-monetary.c b/locale/programs/ld-monetary.c
index 277b9ff042..3b0412b405 100644
--- a/locale/programs/ld-monetary.c
+++ b/locale/programs/ld-monetary.c
@@ -207,7 +207,6 @@ No definition for %s category found"), "LC_MONETARY");
 
   TEST_ELEM (int_curr_symbol, "");
   TEST_ELEM (currency_symbol, "");
-  TEST_ELEM (mon_decimal_point, ".");
   TEST_ELEM (mon_thousands_sep, "");
   TEST_ELEM (positive_sign, "");
   TEST_ELEM (negative_sign, "");
@@ -257,6 +256,7 @@ not correspond to a valid name in ISO 4217 [--no-warnings=intcurrsym]"),
 	record_error (0, 0, _("%s: field `%s' not defined"),
 		      "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)
     {
@@ -264,8 +264,6 @@ not correspond to a valid name in ISO 4217 [--no-warnings=intcurrsym]"),
 %s: value for field `%s' must not be an empty string"),
 		    "LC_MONETARY", "mon_decimal_point");
     }
-  if (monetary->mon_decimal_point_wc == L'\0')
-    monetary->mon_decimal_point_wc = L'.';
 
   if (monetary->mon_grouping_len == 0)
     {


                 reply	other threads:[~2022-02-01 17:07 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220201170733.49A22385AC1B@sourceware.org \
    --to=carlos@sourceware.org \
    --cc=glibc-cvs@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).