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

Message ID ne03t8$nla$1@ger.gmane.org
State Superseded
Headers

Commit Message

Stefan Liebler April 5, 2016, 10:26 a.m. UTC
  On 04/04/2016 04:20 PM, Florian Weimer wrote:
> On 03/30/2016 05:08 PM, Martin Sebor wrote:
>> On 03/30/2016 05:52 AM, Florian Weimer wrote:
>>> On 03/07/2016 09:05 PM, Florian Weimer wrote:
>>>> On 03/07/2016 05:58 PM, Carlos O'Donell wrote:
>>>>> On 03/07/2016 10:58 AM, Florian Weimer wrote:
>>>>>> On 03/07/2016 04:28 PM, Carlos O'Donell wrote:
>>>>>>
>>>>>>> I tend to agree with Martin here, having that separation between
>>>>>>> test and data makes it easier to read and change the test or add
>>>>>>> more tests.
>>>>>>>
>>>>>>> If you're getting a warning from the compiler you expect but don't
>>>>>>> care about then you can just silence the warning with the appropriate
>>>>>>> attribute?
>>>>>>
>>>>>> The warning is not enabled by default (or even -W), so I'm not sure if
>>>>>> that's even necessary.
>>>>>
>>>>> If the warning isn't enabled, then we don't need to worry about it
>>>>> today.
>>>>> The vision here is that we can run everything with -Werror, but we
>>>>> aren't
>>>>> there yet in some cases (see -Wundef fixes required).
>>>>
>>>> Okay, here is a table-based version of the test.  Other parts of the
>>>> patch are unchanged.
>>>
>>> Ping?
>>>
>>>     https://sourceware.org/ml/libc-alpha/2016-03/msg00176.html
>>
>> I like it.
>
> Thanks, committed.
>
> Florian
>
>

Hi,

on s390, i get elf/check-abi-libc.out:
--- ../sysdeps/unix/sysv/linux/s390/s390-64/libc.abilist 
2016-03-04 14:08:24.443047376 +0100
+++ /home/stli/glibcDir/glibc-20160405-build/libc.symlist 
2016-04-05 08:34:18.215255347 +0200
@@ -2131,0 +2132 @@ GLIBC_2.4 __printf_fp F
+GLIBC_2.4 __printf_fp_l F


In stdio-common/printf_fp.c ldbl_-macros are used for __printf_fp_l ...:
ldbl_hidden_def (___printf_fp_l, __printf_fp_l)
ldbl_strong_alias (___printf_fp_l, __printf_fp_l)


... which are defined in sysdeps/generic/math_ldbl_opt.h
(e.g. used on x86_64):
#define ldbl_hidden_def(local, name) libc_hidden_def (name)
#define ldbl_strong_alias(name, aliasname) strong_alias (name, aliasname)


... or sysdeps/ieee754/ldbl-opt/math_ldbl_opt.h
(e.g used on s390):
#ifdef SHARED
# define ldbl_hidden_def(local, name) libc_hidden_ver (local, name)
# define ldbl_strong_alias(name, aliasname) \
   strong_alias (name, __GL_##name##_##aliasname) \
   long_double_symbol (libc, __GL_##name##_##aliasname, aliasname);


The attached patch uses the libc_hidden_def and strong_alias instead of 
ldbl_hidden_def and ldbl_strong_alias macros.

Then, the testsuite is clean on x86_64 and s390.

Please verify.

ChangeLog:

2016-04-05  Stefan Liebler  <stli@linux.vnet.ibm.com>

	* stdio-common/printf_fp.c (__printf_fp_l):
	Use libc_hidden_def and strong_alias instead of
	ldbl_hidden_def and ldbl_strong_alias macros.
  

Comments

Florian Weimer April 5, 2016, 10:53 a.m. UTC | #1
On 04/05/2016 12:26 PM, Stefan Liebler wrote:

> on s390, i get elf/check-abi-libc.out:
> --- ../sysdeps/unix/sysv/linux/s390/s390-64/libc.abilist 2016-03-04
> 14:08:24.443047376 +0100
> +++ /home/stli/glibcDir/glibc-20160405-build/libc.symlist 2016-04-05
> 08:34:18.215255347 +0200
> @@ -2131,0 +2132 @@ GLIBC_2.4 __printf_fp F
> +GLIBC_2.4 __printf_fp_l F
> 
> 
> In stdio-common/printf_fp.c ldbl_-macros are used for __printf_fp_l ...:
> ldbl_hidden_def (___printf_fp_l, __printf_fp_l)
> ldbl_strong_alias (___printf_fp_l, __printf_fp_l)
> 
> 
> ... which are defined in sysdeps/generic/math_ldbl_opt.h
> (e.g. used on x86_64):
> #define ldbl_hidden_def(local, name) libc_hidden_def (name)
> #define ldbl_strong_alias(name, aliasname) strong_alias (name, aliasname)
> 
> 
> ... or sysdeps/ieee754/ldbl-opt/math_ldbl_opt.h
> (e.g used on s390):
> #ifdef SHARED
> # define ldbl_hidden_def(local, name) libc_hidden_ver (local, name)
> # define ldbl_strong_alias(name, aliasname) \
>   strong_alias (name, __GL_##name##_##aliasname) \
>   long_double_symbol (libc, __GL_##name##_##aliasname, aliasname);

Sorry about that.  I asked for guidance on this aspect earlier, but did
not receive any response.

The main question here is if we need additional symbols because we have
a dependency on the double layout, and the code needs to be compiled
twice.  I assume this is the point of the ldbl mechanism, but I'm not sure.

Florian
  
Andreas Schwab April 5, 2016, 12:13 p.m. UTC | #2
sysdeps/ieee754/ldbl-opt needs to be updated to provide
__nldbl___printf_fp_l for -mlong-double-64.

Andreas.
  
Andreas Schwab April 5, 2016, 12:46 p.m. UTC | #3
Please disregard, Stefan's patch is correct.

Andreas.
  
Andreas Schwab April 5, 2016, 12:50 p.m. UTC | #4
Stefan Liebler <stli@linux.vnet.ibm.com> writes:

> diff --git a/stdio-common/printf_fp.c b/stdio-common/printf_fp.c
> index baada9e..fb2a763 100644
> --- a/stdio-common/printf_fp.c
> +++ b/stdio-common/printf_fp.c
> @@ -1248,8 +1248,8 @@ ___printf_fp_l (FILE *fp, locale_t loc,
>    }
>    return done;
>  }
> -ldbl_hidden_def (___printf_fp_l, __printf_fp_l)
> -ldbl_strong_alias (___printf_fp_l, __printf_fp_l)
> +libc_hidden_def (__printf_fp_l)
> +strong_alias (___printf_fp_l, __printf_fp_l)

Please rename ___printf_fp_l to __printf_fp_l, no need for the alias.
Otherwise looks ok.

Andreas.
  

Patch

diff --git a/stdio-common/printf_fp.c b/stdio-common/printf_fp.c
index baada9e..fb2a763 100644
--- a/stdio-common/printf_fp.c
+++ b/stdio-common/printf_fp.c
@@ -1248,8 +1248,8 @@  ___printf_fp_l (FILE *fp, locale_t loc,
   }
   return done;
 }
-ldbl_hidden_def (___printf_fp_l, __printf_fp_l)
-ldbl_strong_alias (___printf_fp_l, __printf_fp_l)
+libc_hidden_def (__printf_fp_l)
+strong_alias (___printf_fp_l, __printf_fp_l)
 
 int
 ___printf_fp (FILE *fp, const struct printf_info *info,