Modernizing pg_bsd_indent's error/warning reporting code - Mailing list pgsql-hackers

Buildfarm member greenfly has recently started to warn about
some very hoary code in pg_bsd_indent [1]:

../pgsql/src/tools/pg_bsd_indent/io.c:562:27: warning: diagnostic behavior may be improved by adding the
'format(printf,2, 3)' attribute to the declaration of 'diag4' [-Wmissing-format-attribute] 
  562 |         fprintf(stdout, msg, a, b);
      |                                  ^
../pgsql/src/tools/pg_bsd_indent/io.c:579:24: warning: diagnostic behavior may be improved by adding the
'format(printf,2, 3)' attribute to the declaration of 'diag3' [-Wmissing-format-attribute] 
  579 |         fprintf(stdout, msg, a);
      |                               ^

This is not an unreasonable suggestion, and presumably more people
will start seeing this as they adopt newer clang versions.  (I see
the same on Fedora 44, for instance.)  So I think we ought to take
the advice, and while we're at it let's convert this code to use
varargs instead of several duplicative functions.  Patch attached.

            regards, tom lane

[1] https://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=greenfly&dt=2026-06-11%2018%3A59%3A42&stg=build

From 2c4f7a3c84f17ea9d855758fb2d0eeae1d2c4a3e Mon Sep 17 00:00:00 2001
From: Tom Lane <tgl@sss.pgh.pa.us>
Date: Fri, 12 Jun 2026 12:46:10 -0400
Subject: [PATCH v1] Modernize pg_bsd_indent's error/warning reporting code.

Late-model clang complains that these functions should be labeled
with "format(printf, 2, 3)", and it's right.  But let's go a bit
further and also make use of varargs, to remove duplication and
allow these functions to be used with non-integer input values.

Since no good deed goes unpunished, I had to also adjust a couple
of call sites.  They weren't wrong as-is, since the size_t-sized
arguments were coerced to int on the way into diag3().  But
without that, we have to adjust the format strings.
---
 src/tools/pg_bsd_indent/indent.c |  4 +--
 src/tools/pg_bsd_indent/indent.h |  9 ++++---
 src/tools/pg_bsd_indent/io.c     | 43 +++++---------------------------
 3 files changed, 14 insertions(+), 42 deletions(-)

diff --git a/src/tools/pg_bsd_indent/indent.c b/src/tools/pg_bsd_indent/indent.c
index 1a29409173b..101554ba5df 100644
--- a/src/tools/pg_bsd_indent/indent.c
+++ b/src/tools/pg_bsd_indent/indent.c
@@ -536,7 +536,7 @@ check_type:
     case lparen:        /* got a '(' or '[' */
         /* count parens to make Healy happy */
         if (++ps.p_l_follow == nitems(ps.paren_indents)) {
-        diag3(0, "Reached internal limit of %d unclosed parens",
+        diag3(0, "Reached internal limit of %zd unclosed parens",
             nitems(ps.paren_indents));
         ps.p_l_follow--;
         }
@@ -809,7 +809,7 @@ check_type:
                          * declaration or an init */
         di_stack[ps.dec_nest] = dec_ind;
         if (++ps.dec_nest == nitems(di_stack)) {
-            diag3(0, "Reached internal limit of %d struct levels",
+            diag3(0, "Reached internal limit of %zd struct levels",
             nitems(di_stack));
             ps.dec_nest--;
         }
diff --git a/src/tools/pg_bsd_indent/indent.h b/src/tools/pg_bsd_indent/indent.h
index e9e71d667d8..974ffe1ac27 100644
--- a/src/tools/pg_bsd_indent/indent.h
+++ b/src/tools/pg_bsd_indent/indent.h
@@ -39,9 +39,7 @@ int    compute_label_target(void);
 int    count_spaces(int, char *);
 int    count_spaces_until(int, char *, char *);
 int    lexi(struct parser_state *);
-void    diag2(int, const char *);
-void    diag3(int, const char *, int);
-void    diag4(int, const char *, int, int);
+void    diag(int level, const char *msg, ...) pg_attribute_printf(2, 3);
 void    dump_line(void);
 int    lookahead(void);
 void    lookahead_reset(void);
@@ -51,3 +49,8 @@ void    pr_comment(void);
 void    set_defaults(void);
 void    set_option(char *);
 void    set_profile(const char *);
+
+/* backwards-compatibility macros */
+#define diag2(level, msg) diag(level, msg)
+#define diag3(level, msg, a) diag(level, msg, a)
+#define diag4(level, msg, a, b) diag(level, msg, a, b)
diff --git a/src/tools/pg_bsd_indent/io.c b/src/tools/pg_bsd_indent/io.c
index 62d600bbb11..787b3ce6177 100644
--- a/src/tools/pg_bsd_indent/io.c
+++ b/src/tools/pg_bsd_indent/io.c
@@ -553,53 +553,22 @@ count_spaces(int cur, char *buffer)
 }

 void
-diag4(int level, const char *msg, int a, int b)
+diag(int level, const char *msg, ...)
 {
-    if (level)
-    found_err = 1;
-    if (output == stdout) {
-    fprintf(stdout, "/**INDENT** %s@%d: ", level == 0 ? "Warning" : "Error", line_no);
-    fprintf(stdout, msg, a, b);
-    fprintf(stdout, " */\n");
-    }
-    else {
-    fprintf(stderr, "%s@%d: ", level == 0 ? "Warning" : "Error", line_no);
-    fprintf(stderr, msg, a, b);
-    fprintf(stderr, "\n");
-    }
-}
-
-void
-diag3(int level, const char *msg, int a)
-{
-    if (level)
-    found_err = 1;
-    if (output == stdout) {
-    fprintf(stdout, "/**INDENT** %s@%d: ", level == 0 ? "Warning" : "Error", line_no);
-    fprintf(stdout, msg, a);
-    fprintf(stdout, " */\n");
-    }
-    else {
-    fprintf(stderr, "%s@%d: ", level == 0 ? "Warning" : "Error", line_no);
-    fprintf(stderr, msg, a);
-    fprintf(stderr, "\n");
-    }
-}
+    va_list ap;

-void
-diag2(int level, const char *msg)
-{
+    va_start(ap, msg);
     if (level)
     found_err = 1;
     if (output == stdout) {
     fprintf(stdout, "/**INDENT** %s@%d: ", level == 0 ? "Warning" : "Error", line_no);
-    fprintf(stdout, "%s", msg);
+    vfprintf(stdout, msg, ap);
     fprintf(stdout, " */\n");
     }
     else {
     fprintf(stderr, "%s@%d: ", level == 0 ? "Warning" : "Error", line_no);
-    fprintf(stderr, "%s", msg);
+    vfprintf(stderr, msg, ap);
     fprintf(stderr, "\n");
     }
+    va_end(ap);
 }
-
--
2.52.0