From: Tom Lane <tgl@sss.pgh.pa.us>
To: pgsql-hackers@lists.postgresql.org
Subject: Modernizing pg_bsd_indent's error/warning reporting code
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="----- =_aaaaaaaaaa0"
Content-ID: <1645027.1781283540.0@sss.pgh.pa.us>
Date: Fri, 12 Jun 2026 12:59:14 -0400
Message-ID: <1645041.1781283554@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/1645041.1781283554%40sss.pgh.pa.us>
Precedence: bulk

------- =_aaaaaaaaaa0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <1645027.1781283540.1@sss.pgh.pa.us>
Content-Transfer-Encoding: quoted-printable

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 dec=
laration 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 dec=
laration 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=3Dgreenf=
ly&dt=3D2026-06-11%2018%3A59%3A42&stg=3Dbuild


------- =_aaaaaaaaaa0
Content-Type: text/x-diff;
	name*0="v1-0001-Modernize-pg_bsd_indent-s-error-warning-reporting.p";
	name*1="atch"; charset="us-ascii"
Content-ID: <1645027.1781283540.2@sss.pgh.pa.us>
Content-Description: 
 v1-0001-Modernize-pg_bsd_indent-s-error-warning-reporting.patch
Content-Transfer-Encoding: quoted-printable

=46rom 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/in=
dent.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 =3D=3D 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] =3D dec_ind;
 		if (++ps.dec_nest =3D=3D 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/in=
dent.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 =3D 1;
-    if (output =3D=3D stdout) {
-	fprintf(stdout, "/**INDENT** %s@%d: ", level =3D=3D 0 ? "Warning" : "Err=
or", line_no);
-	fprintf(stdout, msg, a, b);
-	fprintf(stdout, " */\n");
-    }
-    else {
-	fprintf(stderr, "%s@%d: ", level =3D=3D 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 =3D 1;
-    if (output =3D=3D stdout) {
-	fprintf(stdout, "/**INDENT** %s@%d: ", level =3D=3D 0 ? "Warning" : "Err=
or", line_no);
-	fprintf(stdout, msg, a);
-	fprintf(stdout, " */\n");
-    }
-    else {
-	fprintf(stderr, "%s@%d: ", level =3D=3D 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 =3D 1;
     if (output =3D=3D stdout) {
 	fprintf(stdout, "/**INDENT** %s@%d: ", level =3D=3D 0 ? "Warning" : "Err=
or", line_no);
-	fprintf(stdout, "%s", msg);
+	vfprintf(stdout, msg, ap);
 	fprintf(stdout, " */\n");
     }
     else {
 	fprintf(stderr, "%s@%d: ", level =3D=3D 0 ? "Warning" : "Error", line_no=
);
-	fprintf(stderr, "%s", msg);
+	vfprintf(stderr, msg, ap);
 	fprintf(stderr, "\n");
     }
+    va_end(ap);
 }
-
-- =

2.52.0


------- =_aaaaaaaaaa0--