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 <[email protected]>
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
