Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

Commitd56cb42

Browse files
committed
Activate perlcritic InputOutput::RequireCheckedSyscalls and fix resulting warnings
This checks that certain I/O-related Perl functions properly checktheir return value. Some parts of the PostgreSQL code had been a bitsloppy about that. The new perlcritic warnings are fixed here. Ididn't design any beautiful error messages, mostly just used "or die$!", which mostly matches existing code, and also this isdeveloper-level code, so having the system error plus source codereference should be ok.Initially, we only activate this check for a subset of what theperlcritic check would warn about. The effective list is chmod flock open read rename seek symlink systemThe initial set of functions is picked because most existing codealready checked the return value of those, so any omissions areprobably unintended, or because it seems important for testcorrectness.The actual perlcritic configuration is written as an exclude list.That seems better so that we are clear on what we are currently notchecking. Maybe future patches want to investigate checking some ofthe other functions. (In principle, we might eventually want to checkall of them, but since this is test and build support code, notproduction code, there are probably some reasonable compromises to bemade.)Reviewed-by: Daniel Gustafsson <daniel@yesql.se>Discussion:https://www.postgresql.org/message-id/flat/88b7d4f2-46d9-4cc7-b1f7-613c90f9a76a%40eisentraut.org
1 parentbb5604b commitd56cb42

File tree

15 files changed

+52
-43
lines changed

15 files changed

+52
-43
lines changed

‎src/bin/pg_archivecleanup/t/010_pg_archivecleanup.pl‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ sub create_files
3636
{
3737
foreachmy$fn (map {$_->{name} }@_)
3838
{
39-
openmy$file,'>',"$tempdir/$fn";
39+
openmy$file,'>',"$tempdir/$fn"ordie$!;
4040

4141
print$file'CONTENT';
4242
close$file;

‎src/bin/pg_basebackup/t/010_pg_basebackup.pl‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@
7777
ok(-d"$tempdir/backup",'backup directory was created and left behind');
7878
rmtree("$tempdir/backup");
7979

80-
openmy$conf,'>>',"$pgdata/postgresql.conf";
80+
openmy$conf,'>>',"$pgdata/postgresql.conf"ordie$!;
8181
print$conf"max_replication_slots = 10\n";
8282
print$conf"max_wal_senders = 10\n";
8383
print$conf"wal_level = replica\n";
@@ -175,7 +175,7 @@
175175
qw(backup_label tablespace_map postgresql.auto.conf.tmp
176176
current_logfiles.tmp global/pg_internal.init.123))
177177
{
178-
openmy$file,'>>',"$pgdata/$filename";
178+
openmy$file,'>>',"$pgdata/$filename"ordie$!;
179179
print$file"DONOTCOPY";
180180
close$file;
181181
}
@@ -185,7 +185,7 @@
185185
# unintended side effects.
186186
if ($Config{osname}ne'darwin')
187187
{
188-
openmy$file,'>>',"$pgdata/.DS_Store";
188+
openmy$file,'>>',"$pgdata/.DS_Store"ordie$!;
189189
print$file"DONOTCOPY";
190190
close$file;
191191
}
@@ -423,7 +423,7 @@
423423
my$tblspcoid =$1;
424424
my$escapedRepTsDir =$realRepTsDir;
425425
$escapedRepTsDir =~s/\\/\\\\/g;
426-
openmy$mapfile,'>',$node2->data_dir .'/tablespace_map';
426+
openmy$mapfile,'>',$node2->data_dir .'/tablespace_map'ordie$!;
427427
print$mapfile"$tblspcoid$escapedRepTsDir\n";
428428
close$mapfile;
429429

‎src/bin/pg_ctl/t/001_start_stop.pl‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323
command_ok([$ENV{PG_REGRESS},'--config-auth',"$tempdir/data" ],
2424
'configure authentication');
2525
my$node_port = PostgreSQL::Test::Cluster::get_free_port();
26-
openmy$conf,'>>',"$tempdir/data/postgresql.conf";
26+
openmy$conf,'>>',"$tempdir/data/postgresql.conf"ordie$!;
2727
print$conf"fsync = off\n";
2828
print$conf"port =$node_port\n";
2929
print$conf PostgreSQL::Test::Utils::slurp_file($ENV{TEMP_CONFIG})

‎src/bin/pg_resetwal/t/002_corrupted.pl‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121
my$data;
2222
openmy$fh,'<',$pg_controlor BAIL_OUT($!);
2323
binmode$fh;
24-
read$fh,$data, 16;
24+
read$fh,$data, 16ordie$!;
2525
close$fh;
2626

2727
# Fill pg_control with zeros

‎src/bin/pg_rewind/t/009_growing_files.pl‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@
6969
# Extract the last line from the verbose output as that should have the error
7070
# message for the unexpected file size
7171
my$last;
72-
openmy$f,'<',"$standby_pgdata/tst_both_dir/file1";
72+
openmy$f,'<',"$standby_pgdata/tst_both_dir/file1"ordie$!;
7373
$last =$_while (<$f>);
7474
close$f;
7575
like($last,qr/error: size of source file/,"Check error message");

‎src/bin/pg_rewind/t/RewindTest.pm‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -311,8 +311,8 @@ sub run_pg_rewind
311311
# Make sure that directories have the right umask as this is
312312
# required by a follow-up check on permissions, and better
313313
# safe than sorry.
314-
chmod(0700,$node_primary->archive_dir);
315-
chmod(0700,$node_primary->data_dir ."/pg_wal");
314+
chmod(0700,$node_primary->archive_dir)ordie$!;
315+
chmod(0700,$node_primary->data_dir ."/pg_wal")ordie$!;
316316

317317
# Add appropriate restore_command to the target cluster
318318
$node_primary->enable_restoring($node_primary, 0);

‎src/pl/plperl/text2macro.pl‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,11 +88,11 @@ sub selftest
8888
close$fh;
8989

9090
system("perl$0 --name=X$tmp.pl >$tmp.c") == 0ordie;
91-
open$fh,'>>',"$tmp.c";
91+
open$fh,'>>',"$tmp.c"ordie;
9292
print$fh"#include <stdio.h>\n";
9393
print$fh"int main() { puts(X); return 0; }\n";
9494
close$fh;
95-
system("cat -n$tmp.c");
95+
system("cat -n$tmp.c") == 0ordie;
9696

9797
system("make$tmp") == 0ordie;
9898
open$fh,'<',"./$tmp |"ordie;

‎src/test/kerberos/t/001_auth.pl‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,7 @@
111111
# Construct a pgpass file to make sure we don't use it
112112
append_to_file($pgpass,'*:*:*:*:abc123');
113113

114-
chmod 0600,$pgpass;
114+
chmod 0600,$pgpassordie$!;
115115

116116
# Build the krb5.conf to use.
117117
#

‎src/test/modules/ssl_passphrase_callback/t/001_testfunc.pl‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@
3333
# install certificate and protected key
3434
copy("server.crt",$ddir);
3535
copy("server.key",$ddir);
36-
chmod 0600,"$ddir/server.key";
36+
chmod 0600,"$ddir/server.key"ordie$!;
3737

3838
$node->start;
3939

‎src/test/perl/PostgreSQL/Test/Cluster.pm‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -467,7 +467,7 @@ sub set_replication_conf
467467
$self->hosteq$test_pghost
468468
or croak"set_replication_conf only works with the default host";
469469

470-
openmy$hba,'>>',"$pgdata/pg_hba.conf";
470+
openmy$hba,'>>',"$pgdata/pg_hba.conf"ordie$!;
471471
print$hba
472472
"\n# Allow replication (set up by PostgreSQL::Test::Cluster.pm)\n";
473473
if ($PostgreSQL::Test::Utils::windows_os
@@ -580,7 +580,7 @@ sub init
580580
PostgreSQL::Test::Utils::system_or_bail($ENV{PG_REGRESS},
581581
'--config-auth',$pgdata, @{$params{auth_extra} });
582582

583-
openmy$conf,'>>',"$pgdata/postgresql.conf";
583+
openmy$conf,'>>',"$pgdata/postgresql.conf"ordie$!;
584584
print$conf"\n# Added by PostgreSQL::Test::Cluster.pm\n";
585585
print$conf"fsync = off\n";
586586
print$conf"restart_after_crash = off\n";
@@ -862,7 +862,7 @@ sub init_from_backup
862862
rmdir($data_path);
863863
PostgreSQL::Test::RecursiveCopy::copypath($backup_path,$data_path);
864864
}
865-
chmod(0700,$data_path);
865+
chmod(0700,$data_path)ordie$!;
866866

867867
# Base configuration for this node
868868
$self->append_conf(
@@ -1688,16 +1688,16 @@ sub _reserve_port
16881688
if (kill 0,$pid)
16891689
{
16901690
# process exists and is owned by us, so we can't reserve this port
1691-
flock($portfile, LOCK_UN);
1691+
flock($portfile, LOCK_UN) ||die$!;
16921692
close($portfile);
16931693
return 0;
16941694
}
16951695
}
16961696
# All good, go ahead and reserve the port
1697-
seek($portfile, 0, SEEK_SET);
1697+
seek($portfile, 0, SEEK_SET) ||die$!;
16981698
# print the pid with a fixed width so we don't leave any trailing junk
16991699
print$portfilesprintf("%10d\n",$$);
1700-
flock($portfile, LOCK_UN);
1700+
flock($portfile, LOCK_UN) ||die$!;
17011701
close($portfile);
17021702
push(@port_reservation_files,$filename);
17031703
return 1;

0 commit comments

Comments
 (0)

[8]ページ先頭

©2009-2025 Movatter.jp