From: Tom Lane <tgl@sss.pgh.pa.us>
To: Thomas Munro <thomas.munro@gmail.com>
cc: Tristan Partin <tristan@neon.tech>,
        pgsql-hackers <pgsql-hackers@postgresql.org>
Subject: Re: On non-Windows, hard depend on uselocale(3)
In-reply-to: <664255.1700245108@sss.pgh.pa.us>
References: <CWZBBRR6YA8D.8EHMDRGLCKCD@neon.tech> <2689177.1700070346@sss.pgh.pa.us> <CA+hUKGKo7Q4zayx37JxMaD-BzmkaW9j+k70bhA9auN9jztFrVQ@mail.gmail.com> <2946058.1700081498@sss.pgh.pa.us> <CA+hUKGKu7H7iPNQsZ5FcPkZVbgXc5iTBgUjdpM2EHP4q=Tz42Q@mail.gmail.com> <12842.1700083028@sss.pgh.pa.us> <CA+hUKGJ4s6uDK_KYK-2q=2aWztgvTHYqGeSzugaTCEs6_3XqAg@mail.gmail.com> <98824.1700089582@sss.pgh.pa.us> <CA+hUKGLLb1MiqAUwtSb6gQsqm8wgAh+7UEE-6P1tQxjhHCFmrw@mail.gmail.com> <664255.1700245108@sss.pgh.pa.us>
Comments: In-reply-to Tom Lane <tgl@sss.pgh.pa.us>
	message dated "Fri, 17 Nov 2023 13:18:28 -0500"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <810813.1700261928.1@sss.pgh.pa.us>
Content-Transfer-Encoding: quoted-printable
Date: Fri, 17 Nov 2023 17:58:48 -0500
Message-ID: <810814.1700261928@sss.pgh.pa.us>
Archived-At: <https://www.postgresql.org/message-id/810814.1700261928%40sss.pgh.pa.us>
Precedence: bulk

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=3Dformat]
>   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=3Dformat]
>   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.

Wait, I just realized that there's more to this.  ecpglib *does*
rely on our snprintf.c functions:

$ nm --ext --undef src/interfaces/ecpg/ecpglib/*.o | grep printf =

                 U pg_snprintf
                 U pg_fprintf
                 U pg_snprintf
                 U pg_printf
                 U pg_snprintf
                 U pg_sprintf
                 U pg_fprintf
                 U pg_snprintf
                 U pg_vfprintf
                 U pg_snprintf
                 U pg_sprintf
                 U pg_sprintf

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?

In any case, my concern about ecpg_log() is misplaced.
That is really using pg_vfprintf, so it's correctly marked.

			regards, tom lane