This is the mail archive of the mailing list for the glibc project.

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] strfmon_l: Use specified locale for number formatting [BZ #19633]

On 02/25/2016 01:23 PM, Florian Weimer wrote:
strfmon_l incorrectly applied global locale settings to number
formatting because printf_fp could only look at the global locale.

The new test tries to exercise the (in)dependence of a few LC_MONETARY
properties.  Some of the locales I used for testing might be buggy.

I introduced new helper macros to index the locale data in a locale_t
object.  A future cleanup opportunity is the _NL_CURRENT redefinition in
strfmon_l itself.  __guess_grouping should be moved out of printf_fp.c,
with a proper prototype in a header file.

I saw some ldbl_hidden_def thing.  I fear it is used by ld-as-a-functor
magic to work around the lack of templates in C.  What I did should
hopefully be safe.

Martin, does this patch match what you had in mind?

It's very close to what I started with.  I had made some slightly
different choices in my initial approach, some of which would have
probably turned out to be incorrect in the end.  I like that you
added inline functions rather than macros to access the locale
data.  That's a lot cleaner.

The one thing that I'm curious about is the two sets of
_NL_CURRENT_XXX() macros defined in localeinfo.h and controlled
by NL_CURRENT_INDIRECT.  If I'm reading your patch right it
assumes NL_CURRENT_INDIRECT is undefined.  Does it still work
as expected when NL_CURRENT_INDIRECT is defined?  (I never got
far enough along to test this with my patch.)

Other than that, I have one suggestion for the test.  I don't
know if it would be in line with the glibc approach to testing
(and it's also a matter of personal preference), but for what
it's worth, I find data/table driven tests easier to read, work
with, and enhance.  I.e., rather than calling

  check (strfmon_l (actual, sizeof (actual), loc, "%i", 1234567.89),
         "%i", "USD 1,234,567.89");

a bunch of times with varying formats and data, I find that
defining an array of structs containing the data and then
iterating over the data makes it easier to see the important
bits under test and keep the unimportant parts out of sight:

  struct {
      const char *format;
      double value;
      const char *expect;
  } testdata[] = {
      "%i", 1234567.89, "USD 1,234,567.89",

  for (i = 0; i != sizeof testdata / sizeof *testdata; ++i)
      check (strfmon_l (actual, sizeof (actual), loc,
                        tesdata[i].format, testdata[i].value),
                        tesdata[i].format, testdata[i].expect);


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]