[PATCH] Clear up perlcritic 'missing return' warning
This is the first in a series of patches to reduce the number of warnings
from perlcritic. The current plan is to submit a separate patch for each
warning or small set of related warnings, to make reviewing more
straightforward.
This particular patch addresses the warning caused by falling off the end
of a subroutine rather than explicitly returning. It also contains
additions to the perlcritic configuration file to ignore a couple of
warnings that seem acceptable, bringing it in line with the buildfarm's
configuration for those warnings.
Thanks to Andrew for assistance getting my environment set up.
Mike Blackwell
Attachments:
0001-add-returns-to-Perl-files-where-missing.patchtext/x-patch; charset=US-ASCII; name=0001-add-returns-to-Perl-files-where-missing.patchDownload
From d3ec3cbeb496ea9d2b285aaa32359cae528a983b Mon Sep 17 00:00:00 2001
From: Mike Blackwell <maiku41@gmail.com>
Date: Tue, 15 May 2018 16:38:59 -0500
Subject: [PATCH] add returns to Perl files where missing
diff --git a/contrib/bloom/t/001_wal.pl b/contrib/bloom/t/001_wal.pl
index 55f2167d7f..0f2628b557 100644
--- a/contrib/bloom/t/001_wal.pl
+++ b/contrib/bloom/t/001_wal.pl
@@ -36,6 +36,7 @@ SELECT * FROM tst WHERE i = 7 AND t = 'e';
my $standby_result = $node_standby->safe_psql("postgres", $queries);
is($master_result, $standby_result, "$test_name: query result matches");
+ return;
}
# Initialize master node
diff --git a/contrib/intarray/bench/create_test.pl b/contrib/intarray/bench/create_test.pl
index f3262df05b..d2c678bb53 100755
--- a/contrib/intarray/bench/create_test.pl
+++ b/contrib/intarray/bench/create_test.pl
@@ -83,4 +83,5 @@ sub copytable
while (<$fff>) { print; }
close $fff;
print "\\.\n";
+ return;
}
diff --git a/src/backend/catalog/Catalog.pm b/src/backend/catalog/Catalog.pm
index 95bf619c91..ae5b499b6a 100644
--- a/src/backend/catalog/Catalog.pm
+++ b/src/backend/catalog/Catalog.pm
@@ -367,6 +367,7 @@ sub RenameTempFile
{
rename($temp_name, $final_name) || die "rename: $temp_name: $!";
}
+ return;
}
# Find a symbol defined in a particular header file and extract the value.
diff --git a/src/backend/catalog/genbki.pl b/src/backend/catalog/genbki.pl
index ebdc919414..9be51d28b0 100644
--- a/src/backend/catalog/genbki.pl
+++ b/src/backend/catalog/genbki.pl
@@ -649,6 +649,7 @@ sub gen_pg_attribute
}
}
}
+ return;
}
# Given $pgattr_schema (the pg_attribute schema for a catalog sufficient for
@@ -706,6 +707,7 @@ sub morph_row_for_pgattr
}
Catalog::AddDefaultValues($row, $pgattr_schema, 'pg_attribute');
+ return;
}
# Write an entry to postgres.bki.
@@ -744,6 +746,7 @@ sub print_bki_insert
push @bki_values, $bki_value;
}
printf $bki "insert %s( %s )\n", $oid, join(' ', @bki_values);
+ return;
}
# Given a row reference, modify it so that it becomes a valid entry for
@@ -786,6 +789,7 @@ sub morph_row_for_schemapg
# Only the fixed-size portions of the descriptors are ever used.
delete $row->{$attname} if $column->{is_varlen};
}
+ return;
}
# Perform OID lookups on an array of OID names.
diff --git a/src/backend/parser/check_keywords.pl b/src/backend/parser/check_keywords.pl
index ad41e134ac..718441c215 100644
--- a/src/backend/parser/check_keywords.pl
+++ b/src/backend/parser/check_keywords.pl
@@ -18,6 +18,7 @@ sub error
{
print STDERR @_;
$errors = 1;
+ return;
}
$, = ' '; # set output field separator
diff --git a/src/backend/utils/mb/Unicode/convutils.pm b/src/backend/utils/mb/Unicode/convutils.pm
index 69ec099f29..103bd0264e 100644
--- a/src/backend/utils/mb/Unicode/convutils.pm
+++ b/src/backend/utils/mb/Unicode/convutils.pm
@@ -99,6 +99,7 @@ sub print_conversion_tables
$charset);
print_conversion_tables_direction($this_script, $csname, TO_UNICODE,
$charset);
+ return;
}
#############################################################################
@@ -160,6 +161,7 @@ sub print_conversion_tables_direction
}
close($out);
+ return;
}
sub print_from_utf8_combined_map
@@ -194,6 +196,7 @@ sub print_from_utf8_combined_map
}
print $out "\t/* $last_comment */" if ($verbose && $last_comment ne "");
print $out "\n};\n";
+ return;
}
sub print_to_utf8_combined_map
@@ -230,6 +233,7 @@ sub print_to_utf8_combined_map
}
print $out "\t/* $last_comment */" if ($verbose && $last_comment ne "");
print $out "\n};\n";
+ return;
}
#######################################################################
@@ -625,6 +629,7 @@ sub print_radix_table
if ($off != $tblsize) { die "table size didn't match!"; }
print $out "};\n";
+ return;
}
###
diff --git a/src/backend/utils/sort/gen_qsort_tuple.pl b/src/backend/utils/sort/gen_qsort_tuple.pl
index 6186d0a5ba..b6b2ffa7d0 100644
--- a/src/backend/utils/sort/gen_qsort_tuple.pl
+++ b/src/backend/utils/sort/gen_qsort_tuple.pl
@@ -130,6 +130,8 @@ swapfunc(SortTuple *a, SortTuple *b, size_t n)
#define vecswap(a, b, n) if ((n) > 0) swapfunc(a, b, n)
EOM
+
+ return;
}
sub emit_qsort_implementation
@@ -263,4 +265,6 @@ loop:
}
}
EOM
+
+ return;
}
diff --git a/src/bin/pg_archivecleanup/t/010_pg_archivecleanup.pl b/src/bin/pg_archivecleanup/t/010_pg_archivecleanup.pl
index c84674c00a..22782d3042 100644
--- a/src/bin/pg_archivecleanup/t/010_pg_archivecleanup.pl
+++ b/src/bin/pg_archivecleanup/t/010_pg_archivecleanup.pl
@@ -21,6 +21,7 @@ sub create_files
print $file 'CONTENT';
close $file;
}
+ return;
}
create_files();
@@ -89,6 +90,7 @@ sub run_check
"$test_name: newer WAL file was not cleaned up");
ok(-f "$tempdir/unrelated_file",
"$test_name: unrelated file was not cleaned up");
+ return;
}
run_check('', 'pg_archivecleanup');
diff --git a/src/bin/pg_rewind/RewindTest.pm b/src/bin/pg_rewind/RewindTest.pm
index 52531bba7a..60b54119e7 100644
--- a/src/bin/pg_rewind/RewindTest.pm
+++ b/src/bin/pg_rewind/RewindTest.pm
@@ -71,6 +71,7 @@ sub master_psql
system_or_bail 'psql', '-q', '--no-psqlrc', '-d',
$node_master->connstr('postgres'), '-c', "$cmd";
+ return;
}
sub standby_psql
@@ -79,6 +80,7 @@ sub standby_psql
system_or_bail 'psql', '-q', '--no-psqlrc', '-d',
$node_standby->connstr('postgres'), '-c', "$cmd";
+ return;
}
# Run a query against the master, and check that the output matches what's
@@ -112,6 +114,7 @@ sub check_query
$stdout =~ s/\r//g if $Config{osname} eq 'msys';
is($stdout, $expected_stdout, "$test_name: query result matches");
}
+ return;
}
sub setup_cluster
@@ -130,6 +133,7 @@ sub setup_cluster
'postgresql.conf', qq(
wal_keep_segments = 20
));
+ return;
}
sub start_master
@@ -138,6 +142,8 @@ sub start_master
#### Now run the test-specific parts to initialize the master before setting
# up standby
+
+ return;
}
sub create_standby
@@ -162,6 +168,8 @@ recovery_target_timeline='latest'
# The standby may have WAL to apply before it matches the primary. That
# is fine, because no test examines the standby before promotion.
+
+ return;
}
sub promote_standby
@@ -183,6 +191,8 @@ sub promote_standby
# after promotion so quickly that when pg_rewind runs, the standby has not
# performed a checkpoint after promotion yet.
standby_psql("checkpoint");
+
+ return;
}
sub run_pg_rewind
@@ -266,6 +276,8 @@ recovery_target_timeline='latest'
$node_master->start;
#### Now run the test-specific parts to check the result
+
+ return;
}
# Clean up after the test. Stop both servers, if they're still running.
@@ -273,6 +285,7 @@ sub clean_rewind_test
{
$node_master->teardown_node if defined $node_master;
$node_standby->teardown_node if defined $node_standby;
+ return;
}
1;
diff --git a/src/bin/pg_rewind/t/001_basic.pl b/src/bin/pg_rewind/t/001_basic.pl
index 87bb71e8ed..53dbf45be2 100644
--- a/src/bin/pg_rewind/t/001_basic.pl
+++ b/src/bin/pg_rewind/t/001_basic.pl
@@ -97,6 +97,7 @@ in master, before promotion
}
RewindTest::clean_rewind_test();
+ return;
}
# Run the test in both modes
diff --git a/src/bin/pg_rewind/t/002_databases.pl b/src/bin/pg_rewind/t/002_databases.pl
index bef0e173dc..2c9e427831 100644
--- a/src/bin/pg_rewind/t/002_databases.pl
+++ b/src/bin/pg_rewind/t/002_databases.pl
@@ -53,6 +53,7 @@ template1
}
RewindTest::clean_rewind_test();
+ return;
}
# Run the test in both modes.
diff --git a/src/bin/pg_rewind/t/003_extrafiles.pl b/src/bin/pg_rewind/t/003_extrafiles.pl
index 8b469cdae8..496f38c457 100644
--- a/src/bin/pg_rewind/t/003_extrafiles.pl
+++ b/src/bin/pg_rewind/t/003_extrafiles.pl
@@ -81,6 +81,7 @@ sub run_test
"file lists match");
RewindTest::clean_rewind_test();
+ return;
}
# Run the test in both modes.
diff --git a/src/bin/pg_rewind/t/004_pg_xlog_symlink.pl b/src/bin/pg_rewind/t/004_pg_xlog_symlink.pl
index feadaa6a0f..280eceb992 100644
--- a/src/bin/pg_rewind/t/004_pg_xlog_symlink.pl
+++ b/src/bin/pg_rewind/t/004_pg_xlog_symlink.pl
@@ -72,6 +72,7 @@ in standby, after promotion
'table content');
RewindTest::clean_rewind_test();
+ return;
}
# Run the test in both modes
diff --git a/src/bin/pgbench/t/001_pgbench_with_server.pl b/src/bin/pgbench/t/001_pgbench_with_server.pl
index 00fb04fe1e..b9308f7367 100644
--- a/src/bin/pgbench/t/001_pgbench_with_server.pl
+++ b/src/bin/pgbench/t/001_pgbench_with_server.pl
@@ -42,6 +42,8 @@ sub pgbench
# cleanup?
#unlink @filenames or die "cannot unlink files (@filenames): $!";
+
+ return;
}
# Test concurrent insertion into table with UNIQUE oid column. DDL expects
@@ -817,6 +819,7 @@ sub check_pgbench_logs
};
}
ok(unlink(@logs), "remove log files");
+ return;
}
my $bdir = $node->basedir;
diff --git a/src/bin/pgbench/t/002_pgbench_no_server.pl b/src/bin/pgbench/t/002_pgbench_no_server.pl
index a9e067ba4e..aa47710ace 100644
--- a/src/bin/pgbench/t/002_pgbench_no_server.pl
+++ b/src/bin/pgbench/t/002_pgbench_no_server.pl
@@ -24,6 +24,7 @@ sub pgbench
print STDERR "opts=$opts, stat=$stat, out=$out, err=$err, name=$name";
command_checks_all([ 'pgbench', split(/\s+/, $opts) ],
$stat, $out, $err, $name);
+ return;
}
# invoke pgbench with scripts
@@ -48,6 +49,7 @@ sub pgbench_scripts
}
}
command_checks_all(\@cmd, $stat, $out, $err, $name);
+ return;
}
#
diff --git a/src/include/catalog/reformat_dat_file.pl b/src/include/catalog/reformat_dat_file.pl
index 7f6dc6e4c5..687aca0b16 100755
--- a/src/include/catalog/reformat_dat_file.pl
+++ b/src/include/catalog/reformat_dat_file.pl
@@ -207,6 +207,7 @@ sub strip_default_values
{
delete $row->{pronargs} if defined $row->{proargtypes};
}
+ return;
}
# Format the individual elements of a Perl hash into a valid string
diff --git a/src/interfaces/ecpg/preproc/parse.pl b/src/interfaces/ecpg/preproc/parse.pl
index 983c3a3d89..b20383ab17 100644
--- a/src/interfaces/ecpg/preproc/parse.pl
+++ b/src/interfaces/ecpg/preproc/parse.pl
@@ -415,6 +415,7 @@ sub main
}
}
}
+ return;
}
@@ -431,6 +432,7 @@ sub include_file
add_to_buffer($buffer, $_);
}
close($fh);
+ return;
}
sub include_addon
@@ -472,6 +474,7 @@ sub include_addon
sub add_to_buffer
{
push(@{ $buff{ $_[0] } }, "$_[1]\n");
+ return;
}
sub dump_buffer
@@ -480,6 +483,7 @@ sub dump_buffer
print '/* ', $buffer, ' */', "\n";
my $ref = $buff{$buffer};
print @$ref;
+ return;
}
sub dump_fields
@@ -582,6 +586,7 @@ sub dump_fields
add_to_buffer('rules', ' { $$ = NULL; }');
}
}
+ return;
}
@@ -673,4 +678,5 @@ sub preload_addons
push(@{ $x->{lines} }, @code);
}
}
+ return;
}
diff --git a/src/pl/plperl/plc_perlboot.pl b/src/pl/plperl/plc_perlboot.pl
index 05334a662d..f41aa80e80 100644
--- a/src/pl/plperl/plc_perlboot.pl
+++ b/src/pl/plperl/plc_perlboot.pl
@@ -62,6 +62,7 @@ sub ::encode_array_constructor
(my $msg = shift) =~ s/\(eval \d+\) //g;
chomp $msg;
&::elog(&::WARNING, $msg);
+ return;
}
$SIG{__WARN__} = \&plperl_warn;
diff --git a/src/pl/plperl/text2macro.pl b/src/pl/plperl/text2macro.pl
index 27c6ef7e42..52fcbe1be1 100644
--- a/src/pl/plperl/text2macro.pl
+++ b/src/pl/plperl/text2macro.pl
@@ -99,4 +99,5 @@ sub selftest
warn "Test string: $string\n";
warn "Result : $result";
die "Failed!" if $result ne "$string\n";
+ return;
}
diff --git a/src/test/authentication/t/001_password.pl b/src/test/authentication/t/001_password.pl
index c8dc6606be..3a3b0eb7e8 100644
--- a/src/test/authentication/t/001_password.pl
+++ b/src/test/authentication/t/001_password.pl
@@ -31,6 +31,7 @@ sub reset_pg_hba
unlink($node->data_dir . '/pg_hba.conf');
$node->append_conf('pg_hba.conf', "local all all $hba_method");
$node->reload;
+ return;
}
# Test access for a single role, useful to wrap all tests into one.
@@ -47,6 +48,7 @@ sub test_role
my $res = $node->psql('postgres', undef, extra_params => [ '-U', $role ]);
is($res, $expected_res,
"authentication $status_string for method $method, role $role");
+ return;
}
# Initialize master node
diff --git a/src/test/authentication/t/002_saslprep.pl b/src/test/authentication/t/002_saslprep.pl
index e09273edd4..c4b335c45f 100644
--- a/src/test/authentication/t/002_saslprep.pl
+++ b/src/test/authentication/t/002_saslprep.pl
@@ -27,6 +27,7 @@ sub reset_pg_hba
unlink($node->data_dir . '/pg_hba.conf');
$node->append_conf('pg_hba.conf', "local all all $hba_method");
$node->reload;
+ return;
}
# Test access for a single role, useful to wrap all tests into one.
@@ -45,6 +46,7 @@ sub test_login
is($res, $expected_res,
"authentication $status_string for role $role with password $password"
);
+ return;
}
# Initialize master node. Force UTF-8 encoding, so that we can use non-ASCII
diff --git a/src/test/kerberos/t/001_auth.pl b/src/test/kerberos/t/001_auth.pl
index 5e638eb2eb..54f564779d 100644
--- a/src/test/kerberos/t/001_auth.pl
+++ b/src/test/kerberos/t/001_auth.pl
@@ -164,6 +164,7 @@ sub test_access
'-U', $role
]);
is($res, $expected_res, $test_name);
+ return;
}
unlink($node->data_dir . '/pg_hba.conf');
diff --git a/src/test/ldap/t/001_auth.pl b/src/test/ldap/t/001_auth.pl
index 9ade9a2b00..67b406c981 100644
--- a/src/test/ldap/t/001_auth.pl
+++ b/src/test/ldap/t/001_auth.pl
@@ -144,6 +144,7 @@ sub test_access
my $res =
$node->psql('postgres', 'SELECT 1', extra_params => [ '-U', $role ]);
is($res, $expected_res, $test_name);
+ return;
}
note "simple bind";
diff --git a/src/test/perl/PostgresNode.pm b/src/test/perl/PostgresNode.pm
index 82a2611a1e..d12dd60e73 100644
--- a/src/test/perl/PostgresNode.pm
+++ b/src/test/perl/PostgresNode.pm
@@ -372,6 +372,7 @@ sub dump_info
{
my ($self) = @_;
print $self->info;
+ return;
}
@@ -393,6 +394,7 @@ sub set_replication_conf
"host replication all $test_localhost/32 sspi include_realm=1 map=regress\n";
}
close $hba;
+ return;
}
=pod
@@ -487,6 +489,7 @@ sub init
$self->set_replication_conf if $params{allows_streaming};
$self->enable_archiving if $params{has_archiving};
+ return;
}
=pod
@@ -512,6 +515,8 @@ sub append_conf
chmod($self->group_access() ? 0640 : 0600, $conffile)
or die("unable to set permissions for $conffile");
+
+ return;
}
=pod
@@ -538,6 +543,7 @@ sub backup
TestLib::system_or_bail('pg_basebackup', '-D', $backup_path, '-p', $port,
'--no-sync');
print "# Backup finished\n";
+ return;
}
=item $node->backup_fs_hot(backup_name)
@@ -556,6 +562,7 @@ sub backup_fs_hot
{
my ($self, $backup_name) = @_;
$self->_backup_fs($backup_name, 1);
+ return;
}
=item $node->backup_fs_cold(backup_name)
@@ -572,6 +579,7 @@ sub backup_fs_cold
{
my ($self, $backup_name) = @_;
$self->_backup_fs($backup_name, 0);
+ return;
}
@@ -612,6 +620,7 @@ sub _backup_fs
}
print "# Backup finished\n";
+ return;
}
@@ -672,6 +681,7 @@ port = $port
));
$self->enable_streaming($root_node) if $params{has_streaming};
$self->enable_restoring($root_node) if $params{has_restoring};
+ return;
}
=pod
@@ -703,6 +713,7 @@ sub start
}
$self->_update_pid(1);
+ return;
}
=pod
@@ -728,6 +739,7 @@ sub stop
print "### Stopping node \"$name\" using mode $mode\n";
TestLib::system_or_bail('pg_ctl', '-D', $pgdata, '-m', $mode, 'stop');
$self->_update_pid(0);
+ return;
}
=pod
@@ -746,6 +758,7 @@ sub reload
my $name = $self->name;
print "### Reloading node \"$name\"\n";
TestLib::system_or_bail('pg_ctl', '-D', $pgdata, 'reload');
+ return;
}
=pod
@@ -767,6 +780,7 @@ sub restart
TestLib::system_or_bail('pg_ctl', '-D', $pgdata, '-l', $logfile,
'restart');
$self->_update_pid(1);
+ return;
}
=pod
@@ -787,6 +801,7 @@ sub promote
print "### Promoting node \"$name\"\n";
TestLib::system_or_bail('pg_ctl', '-D', $pgdata, '-l', $logfile,
'promote');
+ return;
}
# Internal routine to enable streaming replication on a standby node.
@@ -802,6 +817,7 @@ sub enable_streaming
primary_conninfo='$root_connstr application_name=$name'
standby_mode=on
));
+ return;
}
# Internal routine to enable archive recovery command on a standby node
@@ -830,6 +846,7 @@ sub enable_restoring
restore_command = '$copy_command'
standby_mode = on
));
+ return;
}
# Internal routine to enable archiving
@@ -859,6 +876,7 @@ sub enable_archiving
archive_mode = on
archive_command = '$copy_command'
));
+ return;
}
# Internal method
@@ -885,6 +903,7 @@ sub _update_pid
# Complain if we expected to find a pidfile.
BAIL_OUT("postmaster.pid unexpectedly not present") if $is_running;
+ return;
}
=pod
@@ -1014,7 +1033,7 @@ sub teardown_node
my $self = shift;
$self->stop('immediate');
-
+ return;
}
=pod
@@ -1030,6 +1049,7 @@ sub clean_node
my $self = shift;
rmtree $self->{_basedir} unless defined $self->{_pid};
+ return;
}
=pod
@@ -1351,6 +1371,7 @@ sub command_ok
local $ENV{PGPORT} = $self->port;
TestLib::command_ok(@_);
+ return;
}
=pod
@@ -1368,6 +1389,7 @@ sub command_fails
local $ENV{PGPORT} = $self->port;
TestLib::command_fails(@_);
+ return;
}
=pod
@@ -1385,6 +1407,7 @@ sub command_like
local $ENV{PGPORT} = $self->port;
TestLib::command_like(@_);
+ return;
}
=pod
@@ -1402,6 +1425,7 @@ sub command_checks_all
local $ENV{PGPORT} = $self->port;
TestLib::command_checks_all(@_);
+ return;
}
=pod
@@ -1427,6 +1451,7 @@ sub issues_sql_like
ok($result, "@$cmd exit code 0");
my $log = TestLib::slurp_file($self->logfile);
like($log, $expected_sql, "$test_name: SQL found in server log");
+ return;
}
=pod
@@ -1445,6 +1470,7 @@ sub run_log
local $ENV{PGPORT} = $self->port;
TestLib::run_log(@_);
+ return;
}
=pod
@@ -1548,6 +1574,7 @@ sub wait_for_catchup
$self->poll_query_until('postgres', $query)
or croak "timed out waiting for catchup";
print "done\n";
+ return;
}
=pod
@@ -1590,6 +1617,7 @@ sub wait_for_slot_catchup
$self->poll_query_until('postgres', $query)
or croak "timed out waiting for catchup";
print "done\n";
+ return;
}
=pod
diff --git a/src/test/perl/SimpleTee.pm b/src/test/perl/SimpleTee.pm
index ea2f2ee828..9de7b1ac32 100644
--- a/src/test/perl/SimpleTee.pm
+++ b/src/test/perl/SimpleTee.pm
@@ -13,7 +13,7 @@ use strict;
sub TIEHANDLE
{
my $self = shift;
- bless \@_, $self;
+ return bless \@_, $self;
}
sub PRINT
diff --git a/src/test/perl/TestLib.pm b/src/test/perl/TestLib.pm
index c9f824b4c6..77499c01e9 100644
--- a/src/test/perl/TestLib.pm
+++ b/src/test/perl/TestLib.pm
@@ -194,6 +194,7 @@ sub system_or_bail
{
BAIL_OUT("system $_[0] failed");
}
+ return;
}
sub run_log
@@ -244,6 +245,7 @@ sub append_to_file
or die "could not write \"$filename\": $!";
print $fh $str;
close $fh;
+ return;
}
# Check that all file/dir modes in a directory match the expected values,
@@ -338,6 +340,7 @@ sub chmod_recursive
}
},
$dir);
+ return;
}
# Check presence of a given regexp within pg_config.h for the installation
@@ -366,6 +369,7 @@ sub command_ok
my ($cmd, $test_name) = @_;
my $result = run_log($cmd);
ok($result, $test_name);
+ return;
}
sub command_fails
@@ -373,6 +377,7 @@ sub command_fails
my ($cmd, $test_name) = @_;
my $result = run_log($cmd);
ok(!$result, $test_name);
+ return;
}
sub command_exit_is
@@ -394,6 +399,7 @@ sub command_exit_is
? ($h->full_results)[0]
: $h->result(0);
is($result, $expected, $test_name);
+ return;
}
sub program_help_ok
@@ -406,6 +412,7 @@ sub program_help_ok
ok($result, "$cmd --help exit code 0");
isnt($stdout, '', "$cmd --help goes to stdout");
is($stderr, '', "$cmd --help nothing to stderr");
+ return;
}
sub program_version_ok
@@ -418,6 +425,7 @@ sub program_version_ok
ok($result, "$cmd --version exit code 0");
isnt($stdout, '', "$cmd --version goes to stdout");
is($stderr, '', "$cmd --version nothing to stderr");
+ return;
}
sub program_options_handling_ok
@@ -430,6 +438,7 @@ sub program_options_handling_ok
'2>', \$stderr;
ok(!$result, "$cmd with invalid option nonzero exit code");
isnt($stderr, '', "$cmd with invalid option prints error message");
+ return;
}
sub command_like
@@ -441,6 +450,7 @@ sub command_like
ok($result, "$test_name: exit code 0");
is($stderr, '', "$test_name: no stderr");
like($stdout, $expected_stdout, "$test_name: matches");
+ return;
}
sub command_like_safe
@@ -460,6 +470,7 @@ sub command_like_safe
ok($result, "$test_name: exit code 0");
is($stderr, '', "$test_name: no stderr");
like($stdout, $expected_stdout, "$test_name: matches");
+ return;
}
sub command_fails_like
@@ -470,6 +481,7 @@ sub command_fails_like
my $result = IPC::Run::run $cmd, '>', \$stdout, '2>', \$stderr;
ok(!$result, "$test_name: exit code not 0");
like($stderr, $expected_stderr, "$test_name: matches");
+ return;
}
# Run a command and check its status and outputs.
@@ -509,6 +521,8 @@ sub command_checks_all
{
like($stderr, $re, "$test_name stderr /$re/");
}
+
+ return;
}
1;
diff --git a/src/test/recovery/t/001_stream_rep.pl b/src/test/recovery/t/001_stream_rep.pl
index a29a6c720b..a0d3e8f357 100644
--- a/src/test/recovery/t/001_stream_rep.pl
+++ b/src/test/recovery/t/001_stream_rep.pl
@@ -97,6 +97,8 @@ sub test_target_session_attrs
1,
"connect to node $target_name if mode \"$mode\" and $node1_name,$node2_name listed"
);
+
+ return;
}
# Connect to master in "read-write" mode with master,standby1 list.
@@ -195,6 +197,7 @@ sub replay_check
$node_standby_2->safe_psql('postgres',
qq[SELECT 1 FROM replayed WHERE val = $newval])
or die "standby_2 didn't replay standby_1 value $newval";
+ return;
}
replay_check();
diff --git a/src/test/recovery/t/003_recovery_targets.pl b/src/test/recovery/t/003_recovery_targets.pl
index 824fa4da52..e867479f20 100644
--- a/src/test/recovery/t/003_recovery_targets.pl
+++ b/src/test/recovery/t/003_recovery_targets.pl
@@ -41,6 +41,8 @@ sub test_recovery_standby
# Stop standby node
$node_standby->teardown_node;
+
+ return;
}
# Initialize master node
diff --git a/src/test/recovery/t/007_sync_rep.pl b/src/test/recovery/t/007_sync_rep.pl
index 0ddf70b8b8..bba47da17a 100644
--- a/src/test/recovery/t/007_sync_rep.pl
+++ b/src/test/recovery/t/007_sync_rep.pl
@@ -24,6 +24,7 @@ sub test_sync_state
}
ok($self->poll_query_until('postgres', $check_sql, $expected), $msg);
+ return;
}
# Initialize master node
diff --git a/src/test/recovery/t/009_twophase.pl b/src/test/recovery/t/009_twophase.pl
index 93c22d181c..9ea3bd65fc 100644
--- a/src/test/recovery/t/009_twophase.pl
+++ b/src/test/recovery/t/009_twophase.pl
@@ -20,6 +20,7 @@ sub configure_and_reload
));
$node->psql('postgres', "SELECT pg_reload_conf()", stdout => \$psql_out);
is($psql_out, 't', "reload node $name with $parameter");
+ return;
}
# Set up two nodes, which will alternately be master and replication standby.
diff --git a/src/test/ssl/ServerSetup.pm b/src/test/ssl/ServerSetup.pm
index ced279c31b..1cd3badaa1 100644
--- a/src/test/ssl/ServerSetup.pm
+++ b/src/test/ssl/ServerSetup.pm
@@ -47,6 +47,7 @@ sub test_connect_ok
];
command_ok($cmd, $test_name);
+ return;
}
sub test_connect_fails
@@ -60,6 +61,7 @@ sub test_connect_fails
];
command_fails_like($cmd, $expected_stderr, $test_name);
+ return;
}
# Copy a set of files, taking into account wildcards
@@ -75,6 +77,7 @@ sub copy_files
copy($orig_file, "$dest/$base_file")
or die "Could not copy $orig_file to $dest";
}
+ return;
}
sub configure_test_server_for_ssl
@@ -130,6 +133,8 @@ sub configure_test_server_for_ssl
# Change pg_hba after restart because hostssl requires ssl=on
configure_hba_for_ssl($node, $serverhost, $authmethod);
+
+ return;
}
# Change the configuration to use given server cert file, and reload
@@ -150,6 +155,7 @@ sub switch_server_cert
close $sslconf;
$node->restart;
+ return;
}
sub configure_hba_for_ssl
@@ -173,4 +179,7 @@ sub configure_hba_for_ssl
print $hba
"hostssl certdb all ::1/128 cert\n";
close $hba;
+ return;
}
+
+1;
\ No newline at end of file
diff --git a/src/tools/copyright.pl b/src/tools/copyright.pl
index 41cb93d658..08b9e5b42e 100755
--- a/src/tools/copyright.pl
+++ b/src/tools/copyright.pl
@@ -62,6 +62,7 @@ sub wanted
$line =~ s/$cc (\d{4}), $pgdg/$ccliteral $1-$year, $pgdg/i;
}
untie @lines;
+ return;
}
print "Manually update:\n";
diff --git a/src/tools/git_changelog b/src/tools/git_changelog
index 1262bc104c..af9afcf1f0 100755
--- a/src/tools/git_changelog
+++ b/src/tools/git_changelog
@@ -333,6 +333,7 @@ sub push_commit
push @{ $all_commits_by_branch{ $c->{'branch'} } }, $cc;
$cc->{'branch_position'}{ $c->{'branch'} } =
-1 + @{ $all_commits_by_branch{ $c->{'branch'} } };
+ return;
}
sub hash_commit
@@ -355,6 +356,7 @@ sub parse_datetime
sub output_str
{
($oldest_first) ? ($output_line .= sprintf(shift, @_)) : printf(@_);
+ return;
}
sub output_details
@@ -395,6 +397,7 @@ sub output_details
}
}
output_str("\n");
+ return;
}
sub usage
diff --git a/src/tools/msvc/Install.pm b/src/tools/msvc/Install.pm
index 67124bb109..6f1c30546a 100644
--- a/src/tools/msvc/Install.pm
+++ b/src/tools/msvc/Install.pm
@@ -40,6 +40,7 @@ sub lcopy
copy($src, $target)
|| confess "Could not copy $src to $target\n";
+ return;
}
sub Install
@@ -173,6 +174,7 @@ sub Install
GenerateNLSFiles($target, $config->{nls}, $majorver) if ($config->{nls});
print "Installation complete.\n";
+ return;
}
sub EnsureDirectories
@@ -183,6 +185,7 @@ sub EnsureDirectories
{
mkdir $target . '/' . $d unless -d ($target . '/' . $d);
}
+ return;
}
sub CopyFiles
@@ -200,6 +203,7 @@ sub CopyFiles
lcopy($f, $target . basename($f));
}
print "\n";
+ return;
}
sub CopySetOfFiles
@@ -215,6 +219,7 @@ sub CopySetOfFiles
lcopy($_, $tgt) || croak "Could not copy $_: $!\n";
}
print "\n";
+ return;
}
sub CopySolutionOutput
@@ -340,6 +345,7 @@ sub CopySolutionOutput
print ".";
}
print "\n";
+ return;
}
sub GenerateConversionScript
@@ -377,6 +383,7 @@ sub GenerateConversionScript
print $F $sql;
close($F);
print "\n";
+ return;
}
sub GenerateTimezoneFiles
@@ -408,6 +415,7 @@ sub GenerateTimezoneFiles
system(@args);
print "\n";
+ return;
}
sub GenerateTsearchFiles
@@ -449,6 +457,7 @@ sub GenerateTsearchFiles
}
close($F);
print "\n";
+ return;
}
sub CopyContribFiles
@@ -475,6 +484,7 @@ sub CopyContribFiles
}
}
print "\n";
+ return;
}
sub CopySubdirFiles
@@ -561,6 +571,7 @@ sub CopySubdirFiles
print '.';
}
}
+ return;
}
sub ParseAndCleanRule
@@ -676,6 +687,7 @@ sub CopyIncludeFiles
$target . '/include/informix/esql/',
'src/interfaces/ecpg/include/',
split /\s+/, $1);
+ return;
}
sub GenerateNLSFiles
@@ -719,6 +731,7 @@ sub GenerateNLSFiles
}
}
print "\n";
+ return;
}
sub DetermineMajorVersion
diff --git a/src/tools/msvc/MSBuildProject.pm b/src/tools/msvc/MSBuildProject.pm
index 2726d603af..27397ba3fb 100644
--- a/src/tools/msvc/MSBuildProject.pm
+++ b/src/tools/msvc/MSBuildProject.pm
@@ -80,6 +80,7 @@ EOF
strpool => 'true',
runtime => 'MultiThreadedDLL'
});
+ return;
}
sub AddDefine
@@ -87,6 +88,7 @@ sub AddDefine
my ($self, $def) = @_;
$self->{defines} .= $def . ';';
+ return;
}
sub WriteReferences
@@ -112,6 +114,7 @@ EOF
</ItemGroup>
EOF
}
+ return;
}
sub WriteFiles
@@ -223,6 +226,7 @@ EOF
</ItemGroup>
EOF
}
+ return;
}
sub WriteConfigurationHeader
@@ -234,6 +238,7 @@ sub WriteConfigurationHeader
<Platform>$self->{platform}</Platform>
</ProjectConfiguration>
EOF
+ return;
}
sub WriteConfigurationPropertyGroup
@@ -252,6 +257,7 @@ sub WriteConfigurationPropertyGroup
<WholeProgramOptimization>$p->{wholeopt}</WholeProgramOptimization>
</PropertyGroup>
EOF
+ return;
}
sub WritePropertySheetsPropertyGroup
@@ -262,6 +268,7 @@ sub WritePropertySheetsPropertyGroup
<Import Project="\$(UserRootDir)\\Microsoft.Cpp.\$(Platform).user.props" Condition="exists('\$(UserRootDir)\\Microsoft.Cpp.\$(Platform).user.props')" Label="LocalAppDataPlatform" />
</ImportGroup>
EOF
+ return;
}
sub WriteAdditionalProperties
@@ -272,6 +279,7 @@ sub WriteAdditionalProperties
<IntDir Condition="'\$(Configuration)|\$(Platform)'=='$cfgname|$self->{platform}'">.\\$cfgname\\$self->{name}\\</IntDir>
<LinkIncremental Condition="'\$(Configuration)|\$(Platform)'=='$cfgname|$self->{platform}'">false</LinkIncremental>
EOF
+ return;
}
sub WriteItemDefinitionGroup
@@ -364,6 +372,7 @@ EOF
print $f <<EOF;
</ItemDefinitionGroup>
EOF
+ return;
}
sub Footer
@@ -377,6 +386,7 @@ sub Footer
</ImportGroup>
</Project>
EOF
+ return;
}
package VC2010Project;
@@ -441,6 +451,7 @@ sub WriteConfigurationPropertyGroup
<PlatformToolset>$self->{PlatformToolset}</PlatformToolset>
</PropertyGroup>
EOF
+ return;
}
package VC2013Project;
diff --git a/src/tools/msvc/Mkvcbuild.pm b/src/tools/msvc/Mkvcbuild.pm
index d0d7d46495..4543d87d83 100644
--- a/src/tools/msvc/Mkvcbuild.pm
+++ b/src/tools/msvc/Mkvcbuild.pm
@@ -964,6 +964,7 @@ sub AddContrib
# Are there any output data files to build?
GenerateContribSqlFiles($n, $mf);
+ return;
}
sub GenerateContribSqlFiles
@@ -1010,6 +1011,7 @@ sub GenerateContribSqlFiles
}
}
}
+ return;
}
sub AdjustContribProj
@@ -1020,6 +1022,7 @@ sub AdjustContribProj
\@contrib_uselibpq, \@contrib_uselibpgport,
\@contrib_uselibpgcommon, $contrib_extralibs,
$contrib_extrasource, $contrib_extraincludes);
+ return;
}
sub AdjustFrontendProj
@@ -1030,6 +1033,7 @@ sub AdjustFrontendProj
\@frontend_uselibpq, \@frontend_uselibpgport,
\@frontend_uselibpgcommon, $frontend_extralibs,
$frontend_extrasource, $frontend_extraincludes);
+ return;
}
sub AdjustModule
@@ -1086,6 +1090,7 @@ sub AdjustModule
$proj->AddFile($i);
}
}
+ return;
}
END
diff --git a/src/tools/msvc/Project.pm b/src/tools/msvc/Project.pm
index 46c680d536..261c913ea0 100644
--- a/src/tools/msvc/Project.pm
+++ b/src/tools/msvc/Project.pm
@@ -45,6 +45,7 @@ sub AddFile
my ($self, $filename) = @_;
$self->{files}->{$filename} = 1;
+ return;
}
sub AddFiles
@@ -56,6 +57,7 @@ sub AddFiles
{
$self->{files}->{ $dir . "/" . $f } = 1;
}
+ return;
}
sub ReplaceFile
@@ -110,6 +112,7 @@ sub RelocateFiles
$self->AddFile($targetdir . '/' . basename($f));
}
}
+ return;
}
sub AddReference
@@ -122,6 +125,7 @@ sub AddReference
$self->AddLibrary(
"__CFGNAME__/" . $ref->{name} . "/" . $ref->{name} . ".lib");
}
+ return;
}
sub AddLibrary
@@ -138,6 +142,7 @@ sub AddLibrary
{
push @{ $self->{suffixlib} }, $lib;
}
+ return;
}
sub AddIncludeDir
@@ -149,6 +154,7 @@ sub AddIncludeDir
$self->{includes} .= ';';
}
$self->{includes} .= $inc;
+ return;
}
sub AddPrefixInclude
@@ -156,6 +162,7 @@ sub AddPrefixInclude
my ($self, $inc) = @_;
$self->{prefixincludes} = $inc . ';' . $self->{prefixincludes};
+ return;
}
sub AddDefine
@@ -164,6 +171,7 @@ sub AddDefine
$def =~ s/"/""/g;
$self->{defines} .= $def . ';';
+ return;
}
sub FullExportDLL
@@ -173,6 +181,7 @@ sub FullExportDLL
$self->{builddef} = 1;
$self->{def} = "./__CFGNAME__/$self->{name}/$self->{name}.def";
$self->{implib} = "__CFGNAME__/$self->{name}/$libname";
+ return;
}
sub UseDef
@@ -180,6 +189,7 @@ sub UseDef
my ($self, $def) = @_;
$self->{def} = $def;
+ return;
}
sub AddDir
@@ -284,6 +294,7 @@ sub AddDir
}
$self->AddDirResourceFile($reldir);
+ return;
}
# If the directory's Makefile bears a description string, add a resource file.
@@ -299,6 +310,7 @@ sub AddDirResourceFile
if ($mf =~ /^PGAPPICON\s*=\s*(.*)$/m) { $ico = $1; }
$self->AddResourceFile($reldir, $desc, $ico);
}
+ return;
}
sub AddResourceFile
@@ -332,6 +344,7 @@ sub AddResourceFile
close($i);
}
$self->AddFile("$dir/win32ver.rc");
+ return;
}
sub DisableLinkerWarnings
@@ -341,6 +354,7 @@ sub DisableLinkerWarnings
$self->{disablelinkerwarnings} .= ','
unless ($self->{disablelinkerwarnings} eq '');
$self->{disablelinkerwarnings} .= $warnings;
+ return;
}
sub Save
@@ -366,6 +380,7 @@ sub Save
$self->WriteFiles($f);
$self->Footer($f);
close($f);
+ return;
}
sub GetAdditionalLinkerDependencies
diff --git a/src/tools/msvc/Solution.pm b/src/tools/msvc/Solution.pm
index a33e68eccf..8f0b355fc0 100644
--- a/src/tools/msvc/Solution.pm
+++ b/src/tools/msvc/Solution.pm
@@ -75,6 +75,7 @@ sub DeterminePlatform
$? >> 8 == 0 or die "cl command not found";
$self->{platform} = ($output =~ /^\/favor:<.+AMD64/m) ? 'x64' : 'Win32';
print "Detected hardware platform: $self->{platform}\n";
+ return;
}
# Return 1 if $oldfile is newer than $newfile, or if $newfile doesn't exist.
@@ -112,6 +113,7 @@ sub copyFile
}
close($i);
close($o);
+ return;
}
sub GenerateFiles
@@ -533,6 +535,7 @@ EOF
<!ENTITY majorversion "$self->{majorver}">
EOF
close($o);
+ return;
}
sub GenerateDefFile
@@ -555,6 +558,7 @@ sub GenerateDefFile
close($of);
close($if);
}
+ return;
}
sub AddProject
@@ -727,6 +731,7 @@ EOF
EndGlobal
EOF
close($sln);
+ return;
}
sub GetFakeConfigure
diff --git a/src/tools/msvc/VCBuildProject.pm b/src/tools/msvc/VCBuildProject.pm
index 57b8525f3f..03b890b9b7 100644
--- a/src/tools/msvc/VCBuildProject.pm
+++ b/src/tools/msvc/VCBuildProject.pm
@@ -56,6 +56,7 @@ EOF
</Configurations>
EOF
$self->WriteReferences($f);
+ return;
}
sub WriteFiles
@@ -152,6 +153,7 @@ EOF
print $f <<EOF;
</Files>
EOF
+ return;
}
sub Footer
@@ -162,6 +164,7 @@ sub Footer
<Globals/>
</VisualStudioProject>
EOF
+ return;
}
sub WriteConfiguration
@@ -227,6 +230,7 @@ EOF
print $f <<EOF;
</Configuration>
EOF
+ return;
}
sub WriteReferences
@@ -239,6 +243,7 @@ sub WriteReferences
" <ProjectReference ReferencedProjectIdentifier=\"$ref->{guid}\" Name=\"$ref->{name}\" />\n";
}
print $f " </References>\n";
+ return;
}
sub GenerateCustomTool
diff --git a/src/tools/msvc/builddoc.pl b/src/tools/msvc/builddoc.pl
index 8ab7385b9e..f6eb73db21 100644
--- a/src/tools/msvc/builddoc.pl
+++ b/src/tools/msvc/builddoc.pl
@@ -107,7 +107,7 @@ sub renamefiles
move $f, $nf;
}
chdir $savedir;
-
+ return;
}
sub missing
diff --git a/src/tools/msvc/gendef.pl b/src/tools/msvc/gendef.pl
index 9f0cf76fb3..77c3a775b0 100644
--- a/src/tools/msvc/gendef.pl
+++ b/src/tools/msvc/gendef.pl
@@ -20,6 +20,7 @@ sub dumpsyms
system("dumpbin /symbols /out:$tmpfile $_ >NUL")
&& die "Could not call dumpbin";
rename($tmpfile, $symfile);
+ return;
}
# Given a symbol file path, loops over its contents
@@ -116,6 +117,7 @@ sub extract_syms
$def->{ $pieces[6] } = $pieces[3];
}
close($f);
+ return;
}
sub writedef
@@ -143,6 +145,7 @@ sub writedef
}
}
close($fh);
+ return;
}
diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.pl
index a74adbaf99..4dc051aa62 100644
--- a/src/tools/msvc/vcregress.pl
+++ b/src/tools/msvc/vcregress.pl
@@ -111,6 +111,7 @@ sub installcheck
system(@args);
my $status = $? >> 8;
exit $status if $status;
+ return;
}
sub check
@@ -132,6 +133,7 @@ sub check
system(@args);
my $status = $? >> 8;
exit $status if $status;
+ return;
}
sub ecpgcheck
@@ -157,6 +159,7 @@ sub ecpgcheck
system(@args);
$status = $? >> 8;
exit $status if $status;
+ return;
}
sub isolationcheck
@@ -173,6 +176,7 @@ sub isolationcheck
system(@args);
my $status = $? >> 8;
exit $status if $status;
+ return;
}
sub tap_check
@@ -224,6 +228,7 @@ sub bincheck
$mstat ||= $status;
}
exit $mstat if $mstat;
+ return;
}
sub taptest
@@ -244,6 +249,7 @@ sub taptest
InstallTemp();
my $status = tap_check(@args);
exit $status if $status;
+ return;
}
sub mangle_plpython3
@@ -365,6 +371,7 @@ sub plcheck
}
chdir "$topdir";
+ return;
}
sub subdircheck
@@ -413,6 +420,7 @@ sub subdircheck
print join(' ', @args), "\n";
system(@args);
chdir "..";
+ return;
}
sub contribcheck
@@ -434,6 +442,7 @@ sub contribcheck
$mstat ||= $status;
}
exit $mstat if $mstat;
+ return;
}
sub modulescheck
@@ -447,6 +456,7 @@ sub modulescheck
$mstat ||= $status;
}
exit $mstat if $mstat;
+ return;
}
sub recoverycheck
@@ -457,6 +467,7 @@ sub recoverycheck
my $dir = "$topdir/src/test/recovery";
my $status = tap_check($dir);
exit $status if $status;
+ return;
}
# Run "initdb", then reconfigure authentication.
@@ -501,6 +512,7 @@ sub generate_db
system('createdb', quote_system_arg($dbname));
my $status = $? >> 8;
exit $status if $status;
+ return;
}
sub upgradecheck
@@ -586,6 +598,7 @@ sub upgradecheck
print "dumps not identical!\n";
exit(1);
}
+ return;
}
sub fetchRegressOpts
@@ -680,6 +693,7 @@ sub InstallTemp
Install("$tmp_installdir", "all", $config);
}
$ENV{PATH} = "$tmp_installdir/bin;$ENV{PATH}";
+ return;
}
sub usage
diff --git a/src/tools/pginclude/pgcheckdefines b/src/tools/pginclude/pgcheckdefines
index c547df45ea..4edf7fc56e 100755
--- a/src/tools/pginclude/pgcheckdefines
+++ b/src/tools/pginclude/pgcheckdefines
@@ -297,4 +297,6 @@ sub checkit
print "$file references $symbol, defined in @places\n";
# print "includes: @includes\n";
+
+ return;
}
diff --git a/src/tools/pgindent/pgindent b/src/tools/pgindent/pgindent
index fc616e7a08..2d81672e15 100755
--- a/src/tools/pgindent/pgindent
+++ b/src/tools/pgindent/pgindent
@@ -93,6 +93,8 @@ sub check_indent
"You appear to have GNU indent rather than BSD indent.\n";
exit 1;
}
+
+ return;
}
@@ -162,6 +164,7 @@ sub process_exclude
}
close($eh);
}
+ return;
}
@@ -189,6 +192,7 @@ sub write_source
|| die "cannot open file \"$source_filename\": $!\n";
print $src_fh $source;
close($src_fh);
+ return;
}
@@ -316,6 +320,7 @@ sub diff
. $pre_fh->filename . " "
. $post_fh->filename
. " >&2");
+ return;
}
@@ -361,6 +366,7 @@ sub run_build
$ENV{PGINDENT} = abs_path('pg_bsd_indent');
chdir $save_dir;
+ return;
}
@@ -382,6 +388,7 @@ sub build_clean
system("rm -rf src/tools/pgindent/pg_bsd_indent");
system("rm -f src/tools/pgindent/tmp_typedefs.list");
+ return;
}
diff --git a/src/tools/pgperlcritic/perlcriticrc b/src/tools/pgperlcritic/perlcriticrc
index 1059002ad5..6b0ba62916 100644
--- a/src/tools/pgperlcritic/perlcriticrc
+++ b/src/tools/pgperlcritic/perlcriticrc
@@ -6,9 +6,17 @@
#
#####################################################################
-severity = 5
+severity = 4
theme = core
# allow octal constants with leading zeros
[-ValuesAndExpressions::ProhibitLeadingZeros]
+
+[-InputOutput::RequireBriefOpen]
+[-Subroutines::RequireArgUnpacking]
+[-Variables::RequireLocalizedPunctuationVars]
+
+[TestingAndDebugging::ProhibitNoWarnings]
+allow = once
+
diff --git a/src/tools/version_stamp.pl b/src/tools/version_stamp.pl
index 392fd4ae54..499e3deb3a 100755
--- a/src/tools/version_stamp.pl
+++ b/src/tools/version_stamp.pl
@@ -141,4 +141,5 @@ sub sed_file
or die "mv failed: $?";
$fixedfiles .= "\t$filename\n";
+ return;
}
--
2.17.0
On Mon, May 21, 2018 at 08:07:03PM -0500, Mike Blackwell wrote:
This particular patch addresses the warning caused by falling off the end
of a subroutine rather than explicitly returning.
Do we really want to make that a requirement? Making sure that there is
a return clause if a subroutine returns a result makes sense to me, but
making it mandatory if the subroutine does not return anything and
callers don't expect it do is not really a gain in my opinion. And this
maps with any C code.
Just today, I coded a perl subroutine which does not return any
results... This is most likely going to be forgotten.
--
Michael
On Tue, May 22, 2018 at 3:32 AM, Michael Paquier <michael@paquier.xyz>
wrote:
<snip> And this
maps with any C code.
The important differences here are:
*) Declaring a C function as void prevents returning a value. The intent
not to return a value is clear to any caller and is enforced by the
compiler. There is no equivalent protection in Perl.
*) Falling off the end of a C function that returns a type other than
void has undefined results. Perl will always return the value of the last
statement executed.
Because Perl does allow returning a value without explicitly using return,
it's easy to write code that breaks if an unwary person adds a line to the
end of the subroutine. There's a common constructor incantation that has
this problem. It's a real gotcha for C programmers just starting to poke
around in Perl code.
This difference also allows users of .pm modules to abuse the API of a
method intended to be "void", if the value returned falling off the end
happens to seem useful, leading to breakage if the method's code changes in
the future.
This is most likely going to be forgotten.
That's what perlcritic is for. :)
Mike
On Tue, May 22, 2018 at 4:35 PM, Mike Blackwell <maiku41@gmail.com> wrote:
On Tue, May 22, 2018 at 3:32 AM, Michael Paquier <michael@paquier.xyz>
wrote:<snip> And this
maps with any C code.The important differences here are:
*) Declaring a C function as void prevents returning a value. The intent
not to return a value is clear to any caller and is enforced by the
compiler. There is no equivalent protection in Perl.
*) Falling off the end of a C function that returns a type other than void
has undefined results. Perl will always return the value of the last
statement executed.Because Perl does allow returning a value without explicitly using return,
it's easy to write code that breaks if an unwary person adds a line to the
end of the subroutine. There's a common constructor incantation that has
this problem. It's a real gotcha for C programmers just starting to poke
around in Perl code.This difference also allows users of .pm modules to abuse the API of a
method intended to be "void", if the value returned falling off the end
happens to seem useful, leading to breakage if the method's code changes in
the future.This is most likely going to be forgotten.
That's what perlcritic is for. :)
Mike
I should also point out that Mike posted on this subject back on May
11, and nobody but me replied.
And yes, the idea is that if we do this then we adopt a perlcritic
policy that calls it out when we forget.
cheers
andrew
--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On 2018-May-23, Andrew Dunstan wrote:
And yes, the idea is that if we do this then we adopt a perlcritic
policy that calls it out when we forget.
If we can have a buildfarm animal that runs perlcritic that starts green
(and turns yellow with any new critique), with a config that's also part
of our tree, so we can easily change it as we see fit, it should be
good.
--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Wed, May 23, 2018 at 1:45 PM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:
On 2018-May-23, Andrew Dunstan wrote:
And yes, the idea is that if we do this then we adopt a perlcritic
policy that calls it out when we forget.If we can have a buildfarm animal that runs perlcritic that starts green
(and turns yellow with any new critique), with a config that's also part
of our tree, so we can easily change it as we see fit, it should be
good.
Should be completely trivial to do. We already have the core
infrastructure - I added it a week or two ago.
cheers
andrew
--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On 05/23/2018 02:00 PM, Andrew Dunstan wrote:
On Wed, May 23, 2018 at 1:45 PM, Alvaro Herrera
<alvherre@2ndquadrant.com> wrote:On 2018-May-23, Andrew Dunstan wrote:
And yes, the idea is that if we do this then we adopt a perlcritic
policy that calls it out when we forget.If we can have a buildfarm animal that runs perlcritic that starts green
(and turns yellow with any new critique), with a config that's also part
of our tree, so we can easily change it as we see fit, it should be
good.Should be completely trivial to do. We already have the core
infrastructure - I added it a week or two ago.
Not quite trivial but it's done - see
<https://github.com/PGBuildFarm/client-code/commit/92f94ba7df8adbcbdb08f0138d8b5e686611ba1f>.
crake is now set up to run this - see
<https://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=crake&dt=2018-05-26%2014%3A32%3A19&stg=perl-check>
So, are there any other objections?
The patch Mike supplied doesn't give us a clean run (at least on the
machine I tested on), since it turns down the severity level to 4 but
leaves some items unfixed. I propose to enable this policy at level 5
for now, and then remove that when we can go down to level 4 cleanly,
and use its default setting at that stage.
cheers
andrew
--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On 2018-May-26, Andrew Dunstan wrote:
Not quite trivial but it's done - see <https://github.com/PGBuildFarm/client-code/commit/92f94ba7df8adbcbdb08f0138d8b5e686611ba1f>.
crake is now set up to run this - see <https://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=crake&dt=2018-05-26%2014%3A32%3A19&stg=perl-check>
So, are there any other objections?
The patch Mike supplied doesn't give us a clean run (at least on the machine
I tested on), since it turns down the severity level to 4 but leaves some
items unfixed. I propose to enable this policy at level 5 for now, and then
remove that when we can go down to level 4 cleanly, and use its default
setting at that stage.
Just to be clear -- this is done, right? And we plan no further
enhancements in perlcritic area for pg11?
--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On 06/11/2018 12:34 PM, Alvaro Herrera wrote:
On 2018-May-26, Andrew Dunstan wrote:
Not quite trivial but it's done - see <https://github.com/PGBuildFarm/client-code/commit/92f94ba7df8adbcbdb08f0138d8b5e686611ba1f>.
crake is now set up to run this - see <https://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=crake&dt=2018-05-26%2014%3A32%3A19&stg=perl-check>
So, are there any other objections?
The patch Mike supplied doesn't give us a clean run (at least on the machine
I tested on), since it turns down the severity level to 4 but leaves some
items unfixed. I propose to enable this policy at level 5 for now, and then
remove that when we can go down to level 4 cleanly, and use its default
setting at that stage.Just to be clear -- this is done, right? And we plan no further
enhancements in perlcritic area for pg11?
Yes, modulo
/messages/by-id/f3c12e2c-618f-cb6f-082b-a2f604dbe73f@2ndQuadrant.com
I am hoping that we can get the perlcritic severity down to level 3 with
a combination of policy settings and code remediation during the release
12 dev cycle.
cheers
andrew
--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services