public inbox for [email protected]  

help / color / mirror / Atom feed

From: Tom Lane <[email protected]>
To: Thomas Munro <[email protected]>
Cc: Tristan Partin <[email protected]>
Cc: pgsql-hackers <[email protected]>
Subject: Re: On non-Windows, hard depend on uselocale(3)
Date: Fri, 17 Nov 2023 17:58:48 -0500
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
	<[email protected]>
	<CA+hUKGKo7Q4zayx37JxMaD-BzmkaW9j+k70bhA9auN9jztFrVQ@mail.gmail.com>
	<[email protected]>
	<CA+hUKGKu7H7iPNQsZ5FcPkZVbgXc5iTBgUjdpM2EHP4q=Tz42Q@mail.gmail.com>
	<[email protected]>
	<CA+hUKGJ4s6uDK_KYK-2q=2aWztgvTHYqGeSzugaTCEs6_3XqAg@mail.gmail.com>
	<[email protected]>
	<CA+hUKGLLb1MiqAUwtSb6gQsqm8wgAh+7UEE-6P1tQxjhHCFmrw@mail.gmail.com>
	<[email protected]>

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.

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

view thread (32+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected]
  Subject: Re: On non-Windows, hard depend on uselocale(3)
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox