From: Peter Eisentraut <peter@eisentraut.org>
To: pgsql-committers@lists.postgresql.org
Subject: pgsql: Activate perlcritic InputOutput::RequireCheckedSyscalls and fix
Date: Tue, 19 Mar 2024 06:14:34 +0000
Message-ID: <E1rmSk5-004YFS-SD@gemulon.postgresql.org> (raw)
Activate perlcritic InputOutput::RequireCheckedSyscalls and fix resulting warnings
This checks that certain I/O-related Perl functions properly check
their return value. Some parts of the PostgreSQL code had been a bit
sloppy about that. The new perlcritic warnings are fixed here. I
didn't design any beautiful error messages, mostly just used "or die
$!", which mostly matches existing code, and also this is
developer-level code, so having the system error plus source code
reference should be ok.
Initially, we only activate this check for a subset of what the
perlcritic check would warn about. The effective list is
chmod flock open read rename seek symlink system
The initial set of functions is picked because most existing code
already checked the return value of those, so any omissions are
probably unintended, or because it seems important for test
correctness.
The actual perlcritic configuration is written as an exclude list.
That seems better so that we are clear on what we are currently not
checking. Maybe future patches want to investigate checking some of
the other functions. (In principle, we might eventually want to check
all of them, but since this is test and build support code, not
production code, there are probably some reasonable compromises to be
made.)
Reviewed-by: Daniel Gustafsson <daniel@yesql.se>
Discussion: https://www.postgresql.org/message-id/flat/88b7d4f2-46d9-4cc7-b1f7-613c90f9a76a%40eisentraut.org
Branch
------
master
Details
-------
https://git.postgresql.org/pg/commitdiff/d56cb42b54381d414f1f30929ca267e4768313c8
Modified Files
--------------
src/bin/pg_archivecleanup/t/010_pg_archivecleanup.pl | 2 +-
src/bin/pg_basebackup/t/010_pg_basebackup.pl | 8 ++++----
src/bin/pg_ctl/t/001_start_stop.pl | 2 +-
src/bin/pg_resetwal/t/002_corrupted.pl | 2 +-
src/bin/pg_rewind/t/009_growing_files.pl | 2 +-
src/bin/pg_rewind/t/RewindTest.pm | 4 ++--
src/pl/plperl/text2macro.pl | 4 ++--
src/test/kerberos/t/001_auth.pl | 2 +-
.../modules/ssl_passphrase_callback/t/001_testfunc.pl | 2 +-
src/test/perl/PostgreSQL/Test/Cluster.pm | 12 ++++++------
src/test/perl/PostgreSQL/Test/Utils.pm | 16 ++++++++--------
src/test/ssl/t/SSL/Server.pm | 10 +++++-----
src/tools/msvc_gendef.pl | 4 ++--
src/tools/perlcheck/perlcriticrc | 8 ++++++++
src/tools/pgindent/pgindent | 17 +++++++++--------
15 files changed, 52 insertions(+), 43 deletions(-)
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-committers@postgresql.org
Cc: peter@eisentraut.org, pgsql-committers@lists.postgresql.org
Subject: Re: pgsql: Activate perlcritic InputOutput::RequireCheckedSyscalls and fix
In-Reply-To: <E1rmSk5-004YFS-SD@gemulon.postgresql.org>
* 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