Modernizing pg_bsd_indent's error/warning reporting code

Modernizing pg_bsd_indent's error/warning reporting code

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 
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

Re: Modernizing pg_bsd_indent's error/warning reporting code

Ayush Tiwari  writes:
> On Fri, 12 Jun 2026 at 22:29, Tom Lane  wrote:
>> 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.

> Patch looks good to me,

Thanks for looking!

> Just a minor comment, while we are at it, should we change
> the %zd to %zu for nitems since it looks like it uses size_t?

Seems reasonable enough.  It can't matter functionally, since
the denominators in nitems() are surely at least 2.  But it is
indeed customary to print size_t with %zu, so done that way.

			regards, tom lane

Re: Modernizing pg_bsd_indent's error/warning reporting code