pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Tom Lane <tgl@sss.pgh.pa.us>
To: pgsql-hackers@lists.postgresql.org
Subject: Modernizing pg_bsd_indent's error/warning reporting code
Date: Fri, 12 Jun 2026 12:59:14 -0400
Message-ID: <1645041.1781283554@sss.pgh.pa.us> (raw)

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%3...

Attachments:

v1-0001-Modernize-pg_bsd_indent-s-error-warning-reporting.patch (text/x-diff, 4.3K) download 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

view thread (7+ messages)  latest in thread

Message-ID: <1645041.1781283554@sss.pgh.pa.us>
Permalink:  ../1645041.1781283554@sss.pgh.pa.us/
Also on:    postgresql.org/message-id/1645041.1781283554@sss.pgh.pa.us

 ·  · 

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-hackers@postgresql.org
  Cc: tgl@sss.pgh.pa.us, pgsql-hackers@lists.postgresql.org
  Subject: Re: Modernizing pg_bsd_indent's error/warning reporting code
  In-Reply-To: <1645041.1781283554@sss.pgh.pa.us>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox