From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by sourceware.org (Postfix) with ESMTPS id 4E2BE385AC3A for ; Tue, 1 Feb 2022 11:48:01 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 4E2BE385AC3A Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-404-leICZMVMOQ-oA3JGp22E3w-1; Tue, 01 Feb 2022 06:47:54 -0500 X-MC-Unique: leICZMVMOQ-oA3JGp22E3w-1 Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.phx2.redhat.com [10.5.11.22]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id EB442189DF57; Tue, 1 Feb 2022 11:47:53 +0000 (UTC) Received: from oldenburg.str.redhat.com (unknown [10.39.193.205]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 8A1C610840D8; Tue, 1 Feb 2022 11:47:33 +0000 (UTC) From: Florian Weimer To: Carlos O'Donell Cc: libc-alpha@sourceware.org, michael.hudson@canonical.com Subject: Re: [PATCH 1/2] localedef: Fix handling of empty mon_decimal_point References: <20220131053442.3995804-1-carlos@redhat.com> <20220131053442.3995804-2-carlos@redhat.com> Date: Tue, 01 Feb 2022 12:47:31 +0100 In-Reply-To: <20220131053442.3995804-2-carlos@redhat.com> (Carlos O'Donell's message of "Mon, 31 Jan 2022 00:34:41 -0500") Message-ID: <87wnie4xm4.fsf@oldenburg.str.redhat.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.2 (gnu/linux) MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.84 on 10.5.11.22 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain X-Spam-Status: No, score=-12.2 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_DNSWL_LOW, SPF_HELO_NONE, SPF_NONE, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 01 Feb 2022 11:48:02 -0000 * Carlos O'Donell: > 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. > --- > 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) > { I have verified that this does not change the localedef output for the existing locales created by install-locale-files. I think we need further cleanups in the comments and checks (which were coped from LC_NUMERIC, but should not apply to LC_MONETARY). But I think we can release with this version. Reviewed-by: Florian Weimer Thanks, Florian