[1/9] Use STRFMON_LDBL_IS_DBL instead of __ldbl_is_dbl.

Message ID 20180307193205.4751-2-zackw@panix.com
State Superseded
Headers

Commit Message

Zack Weinberg March 7, 2018, 7:31 p.m. UTC
  This patch takes the first step toward removing the global flag
__ldbl_is_dbl, creating a __vstrfmon_l_internal that takes a flags
parameter instead.

This change arguably makes the generated code slightly worse on
architectures where __ldbl_is_dbl is never true; right now, on those
architectures, it's a compile-time constant; after this change, the
compiler could theoretically prove that __vstrfmon_l_internal was
never called with a nonzero flags argument, but it would probably need
LTO to do it.  This is not performance critical code and I tend to
think that the maintainability benefits of removing action at a
distance are worth it.  However, we _could_ wrap the runtime flag
check with a macro that was defined to ignore its argument and always
return false on architectures where __ldbl_is_dbl is never true, if
people think the codegen benefits are important.

	* include/monetary.h (STRFMON_LDBL_IS_DBL): New constant.
	(__vstrfmon_l): Rename to __vstrfmon_l_internal and add flags
	argument.
	* stdlib/strfmon_l.c (__vstrfmon_l): Rename to __vstrfmon_l_internal
	and add flags argument.	 Check flags instead of __ldbl_is_dbl when
	deciding whether to set is_long_double.
	(__strfmon_l): Call __vstrfmon_l_internal instead of __vstrfmon_l,
	passing zero for flags argument.
	* stdlib/strfmon.c (strfmon): Same change as made to __strfmon_l.

	* sysdeps/ieee754/ldbl-opt/nldbl-compat.c
	(__nldbl___vstrfmon, __nldbl___vstrfmon_l)
	(__nldbl_strfmon, __nldbl___strfmon_l): Call __vstrfmon_l_internal
	directly, passing STRFMON_LDBL_IS_DBL for flags argument.  Normalize
	variable names.  Remove libc_hidden_def/libc_hidden_proto.
	* sysdeps/ieee754/ldbl-opt/nldbl-compat.h: Don't use NLDBL_DECL
	for __nldbl___vstrfmon_l.

	* manual/locale.texi: Update a reference to vstrfmon_l in comments.
---
 include/monetary.h                      | 10 +++++++---
 manual/locale.texi                      |  9 +++++----
 stdlib/strfmon.c                        |  3 ++-
 stdlib/strfmon_l.c                      |  8 ++++----
 sysdeps/ieee754/ldbl-opt/nldbl-compat.c | 34 ++++++++++++---------------------
 sysdeps/ieee754/ldbl-opt/nldbl-compat.h |  8 +++++---
 6 files changed, 35 insertions(+), 37 deletions(-)
  

Comments

Adhemerval Zanella March 12, 2018, 8:36 p.m. UTC | #1
On 07/03/2018 16:31, Zack Weinberg wrote:
> This patch takes the first step toward removing the global flag
> __ldbl_is_dbl, creating a __vstrfmon_l_internal that takes a flags
> parameter instead.
> 
> This change arguably makes the generated code slightly worse on
> architectures where __ldbl_is_dbl is never true; right now, on those
> architectures, it's a compile-time constant; after this change, the
> compiler could theoretically prove that __vstrfmon_l_internal was
> never called with a nonzero flags argument, but it would probably need
> LTO to do it.  This is not performance critical code and I tend to
> think that the maintainability benefits of removing action at a
> distance are worth it.  However, we _could_ wrap the runtime flag
> check with a macro that was defined to ignore its argument and always
> return false on architectures where __ldbl_is_dbl is never true, if
> people think the codegen benefits are important.
> 
> 	* include/monetary.h (STRFMON_LDBL_IS_DBL): New constant.
> 	(__vstrfmon_l): Rename to __vstrfmon_l_internal and add flags
> 	argument.
> 	* stdlib/strfmon_l.c (__vstrfmon_l): Rename to __vstrfmon_l_internal
> 	and add flags argument.	 Check flags instead of __ldbl_is_dbl when
> 	deciding whether to set is_long_double.
> 	(__strfmon_l): Call __vstrfmon_l_internal instead of __vstrfmon_l,
> 	passing zero for flags argument.
> 	* stdlib/strfmon.c (strfmon): Same change as made to __strfmon_l.
> 
> 	* sysdeps/ieee754/ldbl-opt/nldbl-compat.c
> 	(__nldbl___vstrfmon, __nldbl___vstrfmon_l)
> 	(__nldbl_strfmon, __nldbl___strfmon_l): Call __vstrfmon_l_internal
> 	directly, passing STRFMON_LDBL_IS_DBL for flags argument.  Normalize
> 	variable names.  Remove libc_hidden_def/libc_hidden_proto.
> 	* sysdeps/ieee754/ldbl-opt/nldbl-compat.h: Don't use NLDBL_DECL
> 	for __nldbl___vstrfmon_l.
> 
> 	* manual/locale.texi: Update a reference to vstrfmon_l in comments.

LGTM with a style remark below.

Reviewed-by: Adhemerval Zanella <adhemerval.zanella@linaro.org>

> ---
>  include/monetary.h                      | 10 +++++++---
>  manual/locale.texi                      |  9 +++++----
>  stdlib/strfmon.c                        |  3 ++-
>  stdlib/strfmon_l.c                      |  8 ++++----
>  sysdeps/ieee754/ldbl-opt/nldbl-compat.c | 34 ++++++++++++---------------------
>  sysdeps/ieee754/ldbl-opt/nldbl-compat.h |  8 +++++---
>  6 files changed, 35 insertions(+), 37 deletions(-)
> 
> diff --git a/include/monetary.h b/include/monetary.h
> index c130ed56a3..d12ae03dd3 100644
> --- a/include/monetary.h
> +++ b/include/monetary.h
> @@ -2,7 +2,11 @@
>  #ifndef _ISOMAC
>  #include <stdarg.h>
>  
> -extern ssize_t __vstrfmon_l (char *s, size_t maxsize, locale_t loc,
> -			     const char *format, va_list ap)
> -     attribute_hidden;
> +extern ssize_t __vstrfmon_l_internal (char *s, size_t maxsize, locale_t loc,
> +                                      const char *format, va_list ap,
> +                                      unsigned int flags);
> +
> +/* Flags for __vstrfmon_l_internal.  */
> +#define STRFMON_LDBL_IS_DBL 0x0001
> +
>  #endif
> diff --git a/manual/locale.texi b/manual/locale.texi
> index dabb959f9e..720e0ca952 100644
> --- a/manual/locale.texi
> +++ b/manual/locale.texi
> @@ -1209,10 +1209,11 @@ numbers according to these rules.
>  
>  @deftypefun ssize_t strfmon (char *@var{s}, size_t @var{maxsize}, const char *@var{format}, @dots{})
>  @safety{@prelim{}@mtsafe{@mtslocale{}}@asunsafe{@ascuheap{}}@acunsafe{@acsmem{}}}
> -@c It (and strfmon_l) both call vstrfmon_l, which, besides accessing the
> -@c locale object passed to it, accesses the active locale through
> -@c isdigit (but to_digit assumes ASCII digits only).  It may call
> -@c __printf_fp (@mtslocale @ascuheap @acsmem) and guess_grouping (safe).
> +@c It (and strfmon_l) both call __vstrfmon_l_internal, which, besides
> +@c accessing the locale object passed to it, accesses the active
> +@c locale through isdigit (but to_digit assumes ASCII digits only).
> +@c It may call __printf_fp (@mtslocale @ascuheap @acsmem) and
> +@c guess_grouping (safe).
>  The @code{strfmon} function is similar to the @code{strftime} function
>  in that it takes a buffer, its size, a format string,
>  and values to write into the buffer as text in a form specified
> diff --git a/stdlib/strfmon.c b/stdlib/strfmon.c
> index 01980d3e15..2b742c7ad7 100644
> --- a/stdlib/strfmon.c
> +++ b/stdlib/strfmon.c
> @@ -30,7 +30,8 @@ __strfmon (char *s, size_t maxsize, const char *format, ...)
>  
>    va_start (ap, format);
>  
> -  ssize_t res = __vstrfmon_l (s, maxsize, _NL_CURRENT_LOCALE, format, ap);
> +  ssize_t res = __vstrfmon_l_internal (s, maxsize, _NL_CURRENT_LOCALE,
> +                                       format, ap, 0);
>  
>    va_end (ap);
>  
> diff --git a/stdlib/strfmon_l.c b/stdlib/strfmon_l.c
> index cd3796ced9..f0ebd99bd3 100644
> --- a/stdlib/strfmon_l.c
> +++ b/stdlib/strfmon_l.c
> @@ -76,8 +76,8 @@
>     too.  Some of the information contradicts the information which can
>     be specified in format string.  */
>  ssize_t
> -__vstrfmon_l (char *s, size_t maxsize, locale_t loc, const char *format,
> -	      va_list ap)
> +__vstrfmon_l_internal (char *s, size_t maxsize, locale_t loc,
> +                       const char *format, va_list ap, unsigned int flags)
>  {
>    struct __locale_data *current = loc->__locales[LC_MONETARY];
>    _IO_strfile f;
> @@ -268,7 +268,7 @@ __vstrfmon_l (char *s, size_t maxsize, locale_t loc, const char *format,
>        if (*fmt == 'L')
>  	{
>  	  ++fmt;
> -	  if (!__ldbl_is_dbl)
> +	  if (__glibc_likely ((flags & STRFMON_LDBL_IS_DBL) == 0))
>  	    is_long_double = 1;
>  	}
>  
> @@ -608,7 +608,7 @@ ___strfmon_l (char *s, size_t maxsize, locale_t loc, const char *format, ...)
>  
>    va_start (ap, format);
>  
> -  ssize_t res = __vstrfmon_l (s, maxsize, loc, format, ap);
> +  ssize_t res = __vstrfmon_l_internal (s, maxsize, loc, format, ap, 0);
>  
>    va_end (ap);
>  
> diff --git a/sysdeps/ieee754/ldbl-opt/nldbl-compat.c b/sysdeps/ieee754/ldbl-opt/nldbl-compat.c
> index bf54090d4f..7d19eaba8d 100644
> --- a/sysdeps/ieee754/ldbl-opt/nldbl-compat.c
> +++ b/sysdeps/ieee754/ldbl-opt/nldbl-compat.c
> @@ -50,8 +50,6 @@ libc_hidden_proto (__nldbl___vswprintf_chk)
>  libc_hidden_proto (__nldbl___vasprintf_chk)
>  libc_hidden_proto (__nldbl___vdprintf_chk)
>  libc_hidden_proto (__nldbl___obstack_vprintf_chk)
> -libc_hidden_proto (__nldbl___vstrfmon)
> -libc_hidden_proto (__nldbl___vstrfmon_l)
>  libc_hidden_proto (__nldbl___isoc99_vsscanf)
>  libc_hidden_proto (__nldbl___isoc99_vfscanf)
>  libc_hidden_proto (__nldbl___isoc99_vswscanf)
> @@ -779,12 +777,13 @@ attribute_compat_text_section
>  __nldbl_strfmon (char *s, size_t maxsize, const char *format, ...)
>  {
>    va_list ap;
> -  ssize_t res;
> +  ssize_t ret;
>  
>    va_start (ap, format);
> -  res = __nldbl___vstrfmon (s, maxsize, format, ap);
> +  ret = __vstrfmon_l_internal (s, maxsize, _NL_CURRENT_LOCALE, format, ap,
> +                               STRFMON_LDBL_IS_DBL);
>    va_end (ap);
> -  return res;
> +  return ret;
>  }

I tend to frown at variable names changes such this case, it just add diff
lines without improvement in code readability.

>  
>  ssize_t
> @@ -793,12 +792,13 @@ __nldbl___strfmon_l (char *s, size_t maxsize, locale_t loc,
>  		     const char *format, ...)
>  {
>    va_list ap;
> -  ssize_t res;
> +  ssize_t ret;
>  
>    va_start (ap, format);
> -  res = __nldbl___vstrfmon_l (s, maxsize, loc, format, ap);
> +  ret = __vstrfmon_l_internal (s, maxsize, loc, format, ap,
> +                               STRFMON_LDBL_IS_DBL);
>    va_end (ap);
> -  return res;
> +  return ret;
>  }
>  weak_alias (__nldbl___strfmon_l, __nldbl_strfmon_l)
>  
> @@ -806,28 +806,18 @@ ssize_t
>  attribute_compat_text_section
>  __nldbl___vstrfmon (char *s, size_t maxsize, const char *format, va_list ap)
>  {
> -  ssize_t res;
> -  __no_long_double = 1;
> -  res = __vstrfmon_l (s, maxsize, _NL_CURRENT_LOCALE, format, ap);
> -  __no_long_double = 0;
> -  va_end (ap);
> -  return res;
> +  return __vstrfmon_l_internal (s, maxsize, _NL_CURRENT_LOCALE, format, ap,
> +				STRFMON_LDBL_IS_DBL);
>  }
> -libc_hidden_def (__nldbl___vstrfmon)
>  
>  ssize_t
>  attribute_compat_text_section
>  __nldbl___vstrfmon_l (char *s, size_t maxsize, locale_t loc,
>  		      const char *format, va_list ap)
>  {
> -  ssize_t res;
> -  __no_long_double = 1;
> -  res = __vstrfmon_l (s, maxsize, loc, format, ap);
> -  __no_long_double = 0;
> -  va_end (ap);
> -  return res;
> +  return __vstrfmon_l_internal (s, maxsize, loc, format, ap,
> +				STRFMON_LDBL_IS_DBL);
>  }
> -libc_hidden_def (__nldbl___vstrfmon_l)
>  
>  void
>  attribute_compat_text_section
> diff --git a/sysdeps/ieee754/ldbl-opt/nldbl-compat.h b/sysdeps/ieee754/ldbl-opt/nldbl-compat.h
> index 74f0e459fa..a9a77dce99 100644
> --- a/sysdeps/ieee754/ldbl-opt/nldbl-compat.h
> +++ b/sysdeps/ieee754/ldbl-opt/nldbl-compat.h
> @@ -60,7 +60,6 @@ NLDBL_DECL (vsyslog);
>  NLDBL_DECL (qecvt);
>  NLDBL_DECL (qfcvt);
>  NLDBL_DECL (qgcvt);
> -NLDBL_DECL (__vstrfmon_l);
>  NLDBL_DECL (__isoc99_scanf);
>  NLDBL_DECL (__isoc99_fscanf);
>  NLDBL_DECL (__isoc99_sscanf);
> @@ -74,10 +73,13 @@ NLDBL_DECL (__isoc99_vwscanf);
>  NLDBL_DECL (__isoc99_vfwscanf);
>  NLDBL_DECL (__isoc99_vswscanf);
>  
> -/* This one does not exist in the normal interface, only
> -   __nldbl___vstrfmon really exists.  */
> +/* These do not exist in the normal interface, but must exist in the
> +   __nldbl interface so that they can be called from libnldbl.  */
>  extern ssize_t __nldbl___vstrfmon (char *, size_t, const char *, va_list)
>    __THROW;
> +extern ssize_t __nldbl___vstrfmon_l (char *, size_t, locale_t, const char *,
> +				     va_list)
> +  __THROW;
>  
>  /* These don't use __typeof because they were not declared by the headers,
>     since we don't compile with _FORTIFY_SOURCE.  */
>
  
Zack Weinberg March 12, 2018, 9:10 p.m. UTC | #2
On Mon, Mar 12, 2018 at 4:36 PM, Adhemerval Zanella
<adhemerval.zanella@linaro.org> wrote:
>>    va_start (ap, format);
>> -  res = __nldbl___vstrfmon (s, maxsize, format, ap);
>> +  ret = __vstrfmon_l_internal (s, maxsize, _NL_CURRENT_LOCALE, format, ap,
>> +                               STRFMON_LDBL_IS_DBL);
>>    va_end (ap);
>> -  return res;
>> +  return ret;
>>  }
>
> I tend to frown at variable names changes such this case, it just add diff
> lines without improvement in code readability.

This might look pointless by itself, but as you go through the other
patches you will see that many other functions in this file are using
_different_ conventions for their variable names, so making the whole
file be consistent is worthwhile IMHO.  I thought it was better to
touch each function as I was making other changes to it anyway.

zw
  
Adhemerval Zanella March 13, 2018, 11:45 a.m. UTC | #3
On 12/03/2018 18:10, Zack Weinberg wrote:
> On Mon, Mar 12, 2018 at 4:36 PM, Adhemerval Zanella
> <adhemerval.zanella@linaro.org> wrote:
>>>    va_start (ap, format);
>>> -  res = __nldbl___vstrfmon (s, maxsize, format, ap);
>>> +  ret = __vstrfmon_l_internal (s, maxsize, _NL_CURRENT_LOCALE, format, ap,
>>> +                               STRFMON_LDBL_IS_DBL);
>>>    va_end (ap);
>>> -  return res;
>>> +  return ret;
>>>  }
>>
>> I tend to frown at variable names changes such this case, it just add diff
>> lines without improvement in code readability.
> 
> This might look pointless by itself, but as you go through the other
> patches you will see that many other functions in this file are using
> _different_ conventions for their variable names, so making the whole
> file be consistent is worthwhile IMHO.  I thought it was better to
> touch each function as I was making other changes to it anyway.
> 
> zw
> 

Fair enough, this is more a personal preference and this is not really 
a patch blocker (sorry if my ack showed this intention).
  
Gabriel F. T. Gomes March 26, 2018, 3:17 p.m. UTC | #4
On Wed, 07 Mar 2018, Zack Weinberg wrote:

>	* include/monetary.h (STRFMON_LDBL_IS_DBL): New constant.
>	(__vstrfmon_l): Rename to __vstrfmon_l_internal and add flags
>	argument.

Maybe mention that attribute_hidden was not required, thus removed?

Is this another instance of the 'only needed when called both from inside
and outside of glibc' argument (as pointed out in a previous message [1])?

[1] https://sourceware.org/ml/libc-alpha/2018-03/msg00311.html

>-extern ssize_t __vstrfmon_l (char *s, size_t maxsize, locale_t loc,
>-			     const char *format, va_list ap)
>-     attribute_hidden;

Same comment as above, i.e.: is this another instance of the 'only needed
when called both from inside and outside of glibc' argument?

>+extern ssize_t __vstrfmon_l_internal (char *s, size_t maxsize, locale_t loc,
>+                                      const char *format, va_list ap,
>+                                      unsigned int flags);
  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Spaces that could be converted to tabs.


>-  ssize_t res = __vstrfmon_l (s, maxsize, _NL_CURRENT_LOCALE, format, ap);
>+  ssize_t res = __vstrfmon_l_internal (s, maxsize, _NL_CURRENT_LOCALE,
>+                                       format, ap, 0);
  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Likewise.

>-__vstrfmon_l (char *s, size_t maxsize, locale_t loc, const char *format,
>-	      va_list ap)
>+__vstrfmon_l_internal (char *s, size_t maxsize, locale_t loc,
>+                       const char *format, va_list ap, unsigned int flags)
  ~~~~~~~~~~~~~~~~
Likewise.

>-  res = __nldbl___vstrfmon (s, maxsize, format, ap);
>+  ret = __vstrfmon_l_internal (s, maxsize, _NL_CURRENT_LOCALE, format, ap,
>+                               STRFMON_LDBL_IS_DBL);
  ~~~~~~~~~~~~~~~~~~~~~~~~
Likewise.

>-  res = __nldbl___vstrfmon_l (s, maxsize, loc, format, ap);
>+  ret = __vstrfmon_l_internal (s, maxsize, loc, format, ap,
>+                               STRFMON_LDBL_IS_DBL);
  ~~~~~~~~~~~~~~~~~~~~~~~~
Likewise.
  
Zack Weinberg March 26, 2018, 3:40 p.m. UTC | #5
On Mon, Mar 26, 2018 at 11:17 AM, Gabriel F. T. Gomes
<gabriel@inconstante.eti.br> wrote:
> On Wed, 07 Mar 2018, Zack Weinberg wrote:
>
>>       * include/monetary.h (STRFMON_LDBL_IS_DBL): New constant.
>>       (__vstrfmon_l): Rename to __vstrfmon_l_internal and add flags
>>       argument.
>
> Maybe mention that attribute_hidden was not required, thus removed?
>
> Is this another instance of the 'only needed when called both from inside
> and outside of glibc' argument (as pointed out in a previous message [1])?
>
> [1] https://sourceware.org/ml/libc-alpha/2018-03/msg00311.html

I was partially wrong about that, see the subsequent discussion
between me and Florian.  I'm in the process of revising this patchset
to get the hidden annotations 100% correct, and that's sent me down
two levels of rabbit hole...

The rules as I currently understand them are:

 - A function that is _only_ called from inside the DSO that defines
it should have its prototype declaration marked attribute_hidden; the
*_hidden_def and *_hidden_proto macros should not be used.
 - A function that is called from both inside and outside the DSO that
defines it needs two or possibly three names, depending on a bunch of
additional factors; the *_hidden_def and *_hidden_proto macros are for
this case.
 - A function that is _only_ called from _outside_ the DSO that
defines it should not use either attribute_hidden or
*_hidden_{def,proto}.

There are additional complications having to do with static linkage,
whether the function is part of the API exposed to applications,
whether it was specified in C89, and whether we're trying to write
internal code using this function to look like normal (application)
code.  It's a big mess, frankly.

>>+extern ssize_t __vstrfmon_l_internal (char *s, size_t maxsize, locale_t loc,
>>+                                      const char *format, va_list ap,
>>+                                      unsigned int flags);
>   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> Spaces that could be converted to tabs.

Ugh.  I will have to go through the entire patchset and check for
these.  (My editor is set to indent exclusively with spaces, because
several other projects I contribute or have contributed to require
that; glibc is the exception.)

zw
  
Gabriel F. T. Gomes March 26, 2018, 3:52 p.m. UTC | #6
On Mon, 26 Mar 2018, Zack Weinberg wrote:

>Ugh.  I will have to go through the entire patchset and check for
>these.  (My editor is set to indent exclusively with spaces, because
>several other projects I contribute or have contributed to require
>that; glibc is the exception.)

Oh, OK.  Sorry for the similar reports in the other patches...  I won't
report them in the remaining.
  

Patch

diff --git a/include/monetary.h b/include/monetary.h
index c130ed56a3..d12ae03dd3 100644
--- a/include/monetary.h
+++ b/include/monetary.h
@@ -2,7 +2,11 @@ 
 #ifndef _ISOMAC
 #include <stdarg.h>
 
-extern ssize_t __vstrfmon_l (char *s, size_t maxsize, locale_t loc,
-			     const char *format, va_list ap)
-     attribute_hidden;
+extern ssize_t __vstrfmon_l_internal (char *s, size_t maxsize, locale_t loc,
+                                      const char *format, va_list ap,
+                                      unsigned int flags);
+
+/* Flags for __vstrfmon_l_internal.  */
+#define STRFMON_LDBL_IS_DBL 0x0001
+
 #endif
diff --git a/manual/locale.texi b/manual/locale.texi
index dabb959f9e..720e0ca952 100644
--- a/manual/locale.texi
+++ b/manual/locale.texi
@@ -1209,10 +1209,11 @@  numbers according to these rules.
 
 @deftypefun ssize_t strfmon (char *@var{s}, size_t @var{maxsize}, const char *@var{format}, @dots{})
 @safety{@prelim{}@mtsafe{@mtslocale{}}@asunsafe{@ascuheap{}}@acunsafe{@acsmem{}}}
-@c It (and strfmon_l) both call vstrfmon_l, which, besides accessing the
-@c locale object passed to it, accesses the active locale through
-@c isdigit (but to_digit assumes ASCII digits only).  It may call
-@c __printf_fp (@mtslocale @ascuheap @acsmem) and guess_grouping (safe).
+@c It (and strfmon_l) both call __vstrfmon_l_internal, which, besides
+@c accessing the locale object passed to it, accesses the active
+@c locale through isdigit (but to_digit assumes ASCII digits only).
+@c It may call __printf_fp (@mtslocale @ascuheap @acsmem) and
+@c guess_grouping (safe).
 The @code{strfmon} function is similar to the @code{strftime} function
 in that it takes a buffer, its size, a format string,
 and values to write into the buffer as text in a form specified
diff --git a/stdlib/strfmon.c b/stdlib/strfmon.c
index 01980d3e15..2b742c7ad7 100644
--- a/stdlib/strfmon.c
+++ b/stdlib/strfmon.c
@@ -30,7 +30,8 @@  __strfmon (char *s, size_t maxsize, const char *format, ...)
 
   va_start (ap, format);
 
-  ssize_t res = __vstrfmon_l (s, maxsize, _NL_CURRENT_LOCALE, format, ap);
+  ssize_t res = __vstrfmon_l_internal (s, maxsize, _NL_CURRENT_LOCALE,
+                                       format, ap, 0);
 
   va_end (ap);
 
diff --git a/stdlib/strfmon_l.c b/stdlib/strfmon_l.c
index cd3796ced9..f0ebd99bd3 100644
--- a/stdlib/strfmon_l.c
+++ b/stdlib/strfmon_l.c
@@ -76,8 +76,8 @@ 
    too.  Some of the information contradicts the information which can
    be specified in format string.  */
 ssize_t
-__vstrfmon_l (char *s, size_t maxsize, locale_t loc, const char *format,
-	      va_list ap)
+__vstrfmon_l_internal (char *s, size_t maxsize, locale_t loc,
+                       const char *format, va_list ap, unsigned int flags)
 {
   struct __locale_data *current = loc->__locales[LC_MONETARY];
   _IO_strfile f;
@@ -268,7 +268,7 @@  __vstrfmon_l (char *s, size_t maxsize, locale_t loc, const char *format,
       if (*fmt == 'L')
 	{
 	  ++fmt;
-	  if (!__ldbl_is_dbl)
+	  if (__glibc_likely ((flags & STRFMON_LDBL_IS_DBL) == 0))
 	    is_long_double = 1;
 	}
 
@@ -608,7 +608,7 @@  ___strfmon_l (char *s, size_t maxsize, locale_t loc, const char *format, ...)
 
   va_start (ap, format);
 
-  ssize_t res = __vstrfmon_l (s, maxsize, loc, format, ap);
+  ssize_t res = __vstrfmon_l_internal (s, maxsize, loc, format, ap, 0);
 
   va_end (ap);
 
diff --git a/sysdeps/ieee754/ldbl-opt/nldbl-compat.c b/sysdeps/ieee754/ldbl-opt/nldbl-compat.c
index bf54090d4f..7d19eaba8d 100644
--- a/sysdeps/ieee754/ldbl-opt/nldbl-compat.c
+++ b/sysdeps/ieee754/ldbl-opt/nldbl-compat.c
@@ -50,8 +50,6 @@  libc_hidden_proto (__nldbl___vswprintf_chk)
 libc_hidden_proto (__nldbl___vasprintf_chk)
 libc_hidden_proto (__nldbl___vdprintf_chk)
 libc_hidden_proto (__nldbl___obstack_vprintf_chk)
-libc_hidden_proto (__nldbl___vstrfmon)
-libc_hidden_proto (__nldbl___vstrfmon_l)
 libc_hidden_proto (__nldbl___isoc99_vsscanf)
 libc_hidden_proto (__nldbl___isoc99_vfscanf)
 libc_hidden_proto (__nldbl___isoc99_vswscanf)
@@ -779,12 +777,13 @@  attribute_compat_text_section
 __nldbl_strfmon (char *s, size_t maxsize, const char *format, ...)
 {
   va_list ap;
-  ssize_t res;
+  ssize_t ret;
 
   va_start (ap, format);
-  res = __nldbl___vstrfmon (s, maxsize, format, ap);
+  ret = __vstrfmon_l_internal (s, maxsize, _NL_CURRENT_LOCALE, format, ap,
+                               STRFMON_LDBL_IS_DBL);
   va_end (ap);
-  return res;
+  return ret;
 }
 
 ssize_t
@@ -793,12 +792,13 @@  __nldbl___strfmon_l (char *s, size_t maxsize, locale_t loc,
 		     const char *format, ...)
 {
   va_list ap;
-  ssize_t res;
+  ssize_t ret;
 
   va_start (ap, format);
-  res = __nldbl___vstrfmon_l (s, maxsize, loc, format, ap);
+  ret = __vstrfmon_l_internal (s, maxsize, loc, format, ap,
+                               STRFMON_LDBL_IS_DBL);
   va_end (ap);
-  return res;
+  return ret;
 }
 weak_alias (__nldbl___strfmon_l, __nldbl_strfmon_l)
 
@@ -806,28 +806,18 @@  ssize_t
 attribute_compat_text_section
 __nldbl___vstrfmon (char *s, size_t maxsize, const char *format, va_list ap)
 {
-  ssize_t res;
-  __no_long_double = 1;
-  res = __vstrfmon_l (s, maxsize, _NL_CURRENT_LOCALE, format, ap);
-  __no_long_double = 0;
-  va_end (ap);
-  return res;
+  return __vstrfmon_l_internal (s, maxsize, _NL_CURRENT_LOCALE, format, ap,
+				STRFMON_LDBL_IS_DBL);
 }
-libc_hidden_def (__nldbl___vstrfmon)
 
 ssize_t
 attribute_compat_text_section
 __nldbl___vstrfmon_l (char *s, size_t maxsize, locale_t loc,
 		      const char *format, va_list ap)
 {
-  ssize_t res;
-  __no_long_double = 1;
-  res = __vstrfmon_l (s, maxsize, loc, format, ap);
-  __no_long_double = 0;
-  va_end (ap);
-  return res;
+  return __vstrfmon_l_internal (s, maxsize, loc, format, ap,
+				STRFMON_LDBL_IS_DBL);
 }
-libc_hidden_def (__nldbl___vstrfmon_l)
 
 void
 attribute_compat_text_section
diff --git a/sysdeps/ieee754/ldbl-opt/nldbl-compat.h b/sysdeps/ieee754/ldbl-opt/nldbl-compat.h
index 74f0e459fa..a9a77dce99 100644
--- a/sysdeps/ieee754/ldbl-opt/nldbl-compat.h
+++ b/sysdeps/ieee754/ldbl-opt/nldbl-compat.h
@@ -60,7 +60,6 @@  NLDBL_DECL (vsyslog);
 NLDBL_DECL (qecvt);
 NLDBL_DECL (qfcvt);
 NLDBL_DECL (qgcvt);
-NLDBL_DECL (__vstrfmon_l);
 NLDBL_DECL (__isoc99_scanf);
 NLDBL_DECL (__isoc99_fscanf);
 NLDBL_DECL (__isoc99_sscanf);
@@ -74,10 +73,13 @@  NLDBL_DECL (__isoc99_vwscanf);
 NLDBL_DECL (__isoc99_vfwscanf);
 NLDBL_DECL (__isoc99_vswscanf);
 
-/* This one does not exist in the normal interface, only
-   __nldbl___vstrfmon really exists.  */
+/* These do not exist in the normal interface, but must exist in the
+   __nldbl interface so that they can be called from libnldbl.  */
 extern ssize_t __nldbl___vstrfmon (char *, size_t, const char *, va_list)
   __THROW;
+extern ssize_t __nldbl___vstrfmon_l (char *, size_t, locale_t, const char *,
+				     va_list)
+  __THROW;
 
 /* These don't use __typeof because they were not declared by the headers,
    since we don't compile with _FORTIFY_SOURCE.  */