Re: [PATCH] add missing *_unlocked and wcsftime_l prototypes to wchar.h

[Rich Felker <dalias@libc.org>]

On Fri, Sep 09, 2016 at 08:42:45PM -0400, Daniel Sabogal wrote:
> On Fri, Sep 9, 2016 at 7:02 PM, Rich Felker <dalias@libc.org> wrote:
> > On Tue, Sep 06, 2016 at 05:09:39PM -0400, Daniel Sabogal wrote:
> >> these functions had been implemented, but prototypes were not made available
> >> ---
> >>  include/wchar.h               | 12 ++++++++++++
> >>  src/stdio/getwchar.c          |  2 --
> >>  src/stdio/getwchar_unlocked.c |  8 ++++++++
> >>  src/stdio/putwchar.c          |  2 --
> >>  src/stdio/putwchar_unlocked.c |  8 ++++++++
> >>  5 files changed, 28 insertions(+), 4 deletions(-)
> >>  create mode 100644 src/stdio/getwchar_unlocked.c
> >>  create mode 100644 src/stdio/putwchar_unlocked.c
> >>
> >> diff --git a/include/wchar.h b/include/wchar.h
> >> index 0167dce..58818f6 100644
> >> --- a/include/wchar.h
> >> +++ b/include/wchar.h
> >> @@ -136,6 +136,18 @@ size_t wcsftime (wchar_t *__restrict, size_t, const wchar_t *__restrict, const s
> >>
> >>  #undef iswdigit
> >>
> >> +#if defined(_GNU_SOURCE)
> >> +wint_t fgetwc_unlocked (FILE *);
> >> +wint_t getwc_unlocked (FILE *);
> >> +wint_t getwchar_unlocked (void);
> >> +wint_t fputwc_unlocked (wchar_t, FILE *);
> >> +wint_t putwc_unlocked (wchar_t, FILE *);
> >> +wint_t putwchar_unlocked (wchar_t);
> >> +wchar_t *fgetws_unlocked (wchar_t *__restrict, int, FILE *__restrict);
> >> +int fputws_unlocked (const wchar_t *__restrict, FILE *__restrict);
> >> +size_t wcsftime_l (wchar_t *__restrict, size_t, const wchar_t *__restrict, const struct tm *__restrict, locale_t);
> >> +#endif
> >
> > I suspect BSDs also had these so they should possibly be exposed under
> > _GNU_SOURCE || _BSD_SOURCE. Thoughts?
> 
> Glibc doesn't provide any of these with _BSD_SOURCE.
> FreeBSD and NetBSD provide wcsftime_l, but doesn't seem to have the
> *_unlocked variants.
> OpenBSD doesn't seem to provide either.
> 
> Perhaps wcsftime_l should be moved under _GNU_SOURCE || _BSD_SOURCE.

OK.

> >> diff --git a/src/stdio/getwchar.c b/src/stdio/getwchar.c
> >> index bd89e0e..77a9dc1 100644
> >> --- a/src/stdio/getwchar.c
> >> +++ b/src/stdio/getwchar.c
> >> @@ -5,5 +5,3 @@ wint_t getwchar(void)
> >>  {
> >>       return fgetwc(stdin);
> >>  }
> >> -
> >> -weak_alias(getwchar, getwchar_unlocked);
> >> diff --git a/src/stdio/getwchar_unlocked.c b/src/stdio/getwchar_unlocked.c
> >> new file mode 100644
> >> index 0000000..1d00567
> >> --- /dev/null
> >> +++ b/src/stdio/getwchar_unlocked.c
> >> @@ -0,0 +1,8 @@
> >> +#define _GNU_SOURCE
> >> +#include "stdio_impl.h"
> >> +#include <wchar.h>
> >> +
> >> +wint_t getwchar_unlocked(void)
> >> +{
> >> +     return fgetwc_unlocked(stdin);
> >> +}
> >
> > What is the motivation for replacing the aliases with wrappers? This
> > does not seem like an improvement.
> 
> Correct me if I'm wrong, but by using weak_alias(getwchar, getwchar_unlocked)
> instead of the wrapper, wouldn't calls to getwchar_unlocked() really just invoke
> getwchar() or fgetwc(stdin) instead of the intended fgetwc_unlocked(stdin)?

As far as I can tell, the existence of a separate fgetwc_unlocked in
musl is just a historical accident due to its use in implementing
fgetws. It goes back to the initial git check-in and I didn't try to
dig up any older history.

In any case, trying to improve performance of these functions is
orthogonal to exposing declarations for them, so if done at all it
should be a separate commit. But I'd really like to avoid adding
"junk" functions where aliases already work, and get rid of some we
already have.

Rich