Thread: Oddities in our pg_attribute_printf usage

Following my discovery of missed pg_attribute_printf coverage
in libpq_pipeline (cf2c7a736), I went looking to see if we'd
forgotten that anywhere else.  The coverage seems to be solid
... except at the very root, where we have no such markers for
pg_vsnprintf, pg_vsprintf, pg_vfprintf, pg_vprintf.
I wonder if the option to write "0" for the second argument of
pg_attribute_printf didn't exist then, or we didn't know about it?

I'm not sure that adding those markers does all that much for
us, since (a) it's hard to see how the compiler could check
anything if it doesn't have the actual args list at hand,
and (b) these functions are likely never invoked with constant
format strings anyway.  Nonetheless, it seems pretty silly that
we've faithfully labeled all the derivative printf-alikes but
not these fundamental ones.

I also noted that win32security.c thinks it can stick
pg_attribute_printf into a function definition, whereas all
the rest of our code thinks you have to append that marker
to a separate function declaration.  Does that actually work
with Microsoft-platform compilers?  Or more likely, is the
macro expanding to empty on every compiler we've used with
that code?  Anyway, seems like we ought to make this fall
in line with the convention elsewhere.

So I end with the attached.  Any objections?

            regards, tom lane

diff --git a/src/include/port.h b/src/include/port.h
index ce5da0534e..fe48618df7 100644
--- a/src/include/port.h
+++ b/src/include/port.h
@@ -204,13 +204,13 @@ extern unsigned char pg_ascii_tolower(unsigned char ch);
 #undef printf
 #endif

-extern int    pg_vsnprintf(char *str, size_t count, const char *fmt, va_list args);
+extern int    pg_vsnprintf(char *str, size_t count, const char *fmt, va_list args) pg_attribute_printf(3, 0);
 extern int    pg_snprintf(char *str, size_t count, const char *fmt,...) pg_attribute_printf(3, 4);
-extern int    pg_vsprintf(char *str, const char *fmt, va_list args);
+extern int    pg_vsprintf(char *str, const char *fmt, va_list args) pg_attribute_printf(2, 0);
 extern int    pg_sprintf(char *str, const char *fmt,...) pg_attribute_printf(2, 3);
-extern int    pg_vfprintf(FILE *stream, const char *fmt, va_list args);
+extern int    pg_vfprintf(FILE *stream, const char *fmt, va_list args) pg_attribute_printf(2, 0);
 extern int    pg_fprintf(FILE *stream, const char *fmt,...) pg_attribute_printf(2, 3);
-extern int    pg_vprintf(const char *fmt, va_list args);
+extern int    pg_vprintf(const char *fmt, va_list args) pg_attribute_printf(1, 0);
 extern int    pg_printf(const char *fmt,...) pg_attribute_printf(1, 2);

 /*
diff --git a/src/port/win32security.c b/src/port/win32security.c
index 1235199f2f..281244a34f 100644
--- a/src/port/win32security.c
+++ b/src/port/win32security.c
@@ -17,14 +17,14 @@
 #include "postgres_fe.h"
 #endif

+static void log_error(const char *fmt,...) pg_attribute_printf(1, 2);
+

 /*
  * Utility wrapper for frontend and backend when reporting an error
  * message.
  */
-static
-pg_attribute_printf(1, 2)
-void
+static void
 log_error(const char *fmt,...)
 {
     va_list        ap;