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]https://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=greenfly&dt=2026-06-11%2018%3A59%3A42&stg=build:
../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
Attachments:
v1-0001-Modernize-pg_bsd_indent-s-error-warning-reporting.patchtext/x-diff; charset=us-ascii; name*0=v1-0001-Modernize-pg_bsd_indent-s-error-warning-reporting.p; name*1=atchDownload+14-43
Hi,
On Fri, 12 Jun 2026 at 22:29, Tom Lane <tgl@sss.pgh.pa.us> wrote:
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.
Patch looks good to me,
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?
Regards,
Ayush
Ayush Tiwari <ayushtiwari.slg01@gmail.com> writes:
On Fri, 12 Jun 2026 at 22:29, Tom Lane <tgl@sss.pgh.pa.us> 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