Re: On non-Windows, hard depend on uselocale(3) - Mailing list pgsql-hackers

On Sat, Nov 18, 2023 at 11:58 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> I wrote:
> > I've not reviewed this closely, but I did try it on mamba's host.
> > It compiles and passes regression testing, but I see two warnings:
>
> > common.c: In function 'PGTYPESsprintf':
> > common.c:120:2: warning: function 'PGTYPESsprintf' might be a candidate for 'gnu_printf' format attribute
[-Wsuggest-attribute=format]
> >   120 |  return vsprintf_l(str, PGTYPESclocale, format, args);
> >       |  ^~~~~~
> > common.c: In function 'PGTYPESsnprintf':
> > common.c:136:2: warning: function 'PGTYPESsnprintf' might be a candidate for 'gnu_printf' format attribute
[-Wsuggest-attribute=format]
> >   136 |  return vsnprintf_l(str, size, PGTYPESclocale, format, args);
> >       |  ^~~~~~
>
> > I think this is telling us about an actual problem: these new
> > functions are based on libc's printf not what we have in snprintf.c,
> > and therefore we really shouldn't be assuming that they will support
> > any format specs beyond what POSIX requires for printf.

Right, thanks.

> We are getting these warnings because vsprintf_l and
> vsnprintf_l don't have snprintf.c implementations, so the
> compiler sees the attributes attached to them by stdio.h.
>
> This raises the question of whether changing snprintf.c
> could be part of the solution.  I'm not sure that we want
> to try to emulate vs[n]printf_l directly, but perhaps there's
> another way?

Yeah, I have been wondering about that too.

The stuff I posted so far was just about how to remove some gross and
incorrect code from ecpg, a somewhat niche frontend part of
PostgreSQL.  I guess Tristan is thinking bigger: removing obstacles to
going multi-threaded in the backend.  Clearly locales are one of the
places where global state will bite us, so we either need to replace
setlocale() with uselocale() for the database default locale, or use
explicit locale arguments with _l() functions everywhere and pass in
the right locale.  Due to incompleteness of (a) libc implementations
and (b) the standard, we can't directly do either, so we'll need to
cope with that.

Thought experiment:  If we supplied our own fallback _l() replacement
functions where missing, and those did uselocale() save/restore, many
systems wouldn't need them, for example glibc has strtod_l() as you
noted, and several other systems have systematically added them for
all sorts of stuff.  The case of the *printf* family is quite
interesting, because there we already have our own implement for other
reasons, so it might make sense to add the _l() variants to our
snprintf.c implementations.  On glibc, snprintf.c would have to do a
uselocale() save/restore where it punts %g to the system snprintf, but
if that offends some instruction cycle bean counter, perhaps we could
replace that bit with Ryu anyway (or is it not general enough to
handle all the stuff %g et al can do?  I haven't looked).

I am not sure how you would ever figure out what other stuff is
affected by the global locale in general, for example code hiding in
extensions etc, but, I mean, that's what's wrong with global state in
a nutshell and it has often been speculated that multi-threaded
PostgreSQL might have a way to say 'I still want one process per
session because my extensions don't all identify themselves as
thread-safe yet'.

BTW is this comment in snprintf.c true?

 * 1. No locale support: the radix character is always '.' and the '
 * (single quote) format flag is ignored.

It is in the backend but only because we nail down LC_NUMERIC early
on, not because of any property of snprintf.c, no?