Michael Paquier <mich...@paquier.xyz> writes:
> On Fri, May 24, 2024 at 02:09:49PM +0900, Michael Paquier wrote:
>> For now, I have staged for commit the attached, that handles most of
>> the changes from Alexander (msvc could go for more cleanup?).
>
> This one has been applied as of 0c1aca461481 now that v18 is
> open.
>
>> I'll look at the changes from Dagfinn after that, including if perlcritic
>> could be changed. I'll handle the first part when v18 opens up, as
>> that's cosmetic.
For clarity, I've rebased my addional unused-variable changes (except
the errcodes-related ones, see below) onto current master, and split it
into separate commits with detailed explaiations for each file file, see
attached.
> I'm still biased about the second set of changes proposed here,
> though. ProhibitUnusedVariables would have benefits when writing perl
> code in terms of clarity because we would avoid useless stuff, but it
> seems to me that we should put more efforts into the unification of
> the errcodes parsing paths first to have a cleaner long-term picture.
>
> That's not directly the fault of this proposal that we have the same
> parsing rules spread across three PL languages, so perhaps what's
> proposed is fine as-is, at the end.
It turns out there are a couple more places that parse errcodes.txt,
namely doc/src/sgml/generate-errcodes-table.pl and
src/backend/utils/generate-errcodes.pl. I'll have a go refactoring all
of these into a common function à la Catalog::ParseHeader() that returns
a data structure these scripts can use as as appropriate.
> Any thoughts or comments from others more familiar with
> ProhibitUnusedVariables?
Relatedly, I also had a look at prohibiting unused regex captures
(RegularExpressions::ProhibitUnusedCapture), which found a few real
cases, but also lots of false positives in Catalog.pm, because it
doesn't understand that %+ uses all named captures, so I won't propose a
patch for that until that's fixed upstream
(https://github.com/Perl-Critic/Perl-Critic/pull/1065).
- ilmari
>From 8e3c7721da86a670b279bb24b0fe9ae82351628f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Dagfinn=20Ilmari=20Manns=C3=A5ker?= <ilm...@ilmari.org>
Date: Tue, 2 Jul 2024 13:25:16 +0100
Subject: [PATCH 1/3] Remove unused variables in libq encryption negotiation
test
The $node_conf parameter to test_matrix() was only ever used to set
the %params hash (removed in commit d39a49c1e4598048), but that hash
was itself never used for anything, so it (and the $server_config)
hash were always useless.
---
.../libpq/t/005_negotiate_encryption.pl | 24 +++++--------------
1 file changed, 6 insertions(+), 18 deletions(-)
diff --git a/src/interfaces/libpq/t/005_negotiate_encryption.pl b/src/interfaces/libpq/t/005_negotiate_encryption.pl
index c3f70d31bc..251c405926 100644
--- a/src/interfaces/libpq/t/005_negotiate_encryption.pl
+++ b/src/interfaces/libpq/t/005_negotiate_encryption.pl
@@ -215,11 +215,6 @@ BEGIN
my @all_sslmodes = ('disable', 'allow', 'prefer', 'require');
my @all_sslnegotiations = ('postgres', 'direct');
-my $server_config = {
- server_ssl => 0,
- server_gss => 0,
-};
-
###
### Run tests with GSS and SSL disabled in the server
###
@@ -272,7 +267,7 @@ BEGIN
};
note("Running tests with SSL and GSS disabled in the server");
-test_matrix($node, $server_config,
+test_matrix($node,
['testuser'], \@all_gssencmodes, \@all_sslmodes, \@all_sslnegotiations,
parse_table($test_table));
@@ -311,19 +306,15 @@ BEGIN
# Enable SSL in the server
$node->adjust_conf('postgresql.conf', 'ssl', 'on');
$node->reload;
- $server_config->{server_ssl} = 1;
note("Running tests with SSL enabled in server");
- test_matrix(
- $node, $server_config,
- [ 'testuser', 'ssluser', 'nossluser' ], ['disable'],
- \@all_sslmodes, \@all_sslnegotiations,
+ test_matrix($node, [ 'testuser', 'ssluser', 'nossluser' ],
+ ['disable'], \@all_sslmodes, \@all_sslnegotiations,
parse_table($test_table));
# Disable SSL again
$node->adjust_conf('postgresql.conf', 'ssl', 'off');
$node->reload;
- $server_config->{server_ssl} = 0;
}
###
@@ -336,7 +327,6 @@ BEGIN
$krb->create_principal('gssuser', $gssuser_password);
$krb->create_ticket('gssuser', $gssuser_password);
- $server_config->{server_gss} = 1;
$test_table = q{
# USER GSSENCMODE SSLMODE SSLNEGOTIATION EVENTS -> OUTCOME
@@ -400,7 +390,7 @@ BEGIN
}
note("Running tests with GSS enabled in server");
- test_matrix($node, $server_config, [ 'testuser', 'gssuser', 'nogssuser' ],
+ test_matrix($node, [ 'testuser', 'gssuser', 'nogssuser' ],
\@all_gssencmodes, $sslmodes, $sslnegotiations,
parse_table($test_table));
}
@@ -423,7 +413,6 @@ BEGIN
# Enable SSL
$node->adjust_conf('postgresql.conf', 'ssl', 'on');
$node->reload;
- $server_config->{server_ssl} = 1;
$test_table = q{
# USER GSSENCMODE SSLMODE SSLNEGOTIATION EVENTS -> OUTCOME
@@ -510,7 +499,6 @@ BEGIN
note("Running tests with both GSS and SSL enabled in server");
test_matrix(
$node,
- $server_config,
[ 'testuser', 'gssuser', 'ssluser', 'nogssuser', 'nossluser' ],
\@all_gssencmodes,
\@all_sslmodes,
@@ -546,8 +534,8 @@ sub test_matrix
{
local $Test::Builder::Level = $Test::Builder::Level + 1;
- my ($pg_node, $node_conf,
- $test_users, $gssencmodes, $sslmodes, $sslnegotiations, %expected)
+ my ($pg_node, $test_users, $gssencmodes, $sslmodes, $sslnegotiations,
+ %expected)
= @_;
foreach my $test_user (@{$test_users})
--
2.39.2
>From 7127c5a8a45bdb1549fbd99cd37523fc1f848169 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Dagfinn=20Ilmari=20Manns=C3=A5ker?= <ilm...@ilmari.org>
Date: Tue, 2 Jul 2024 13:35:48 +0100
Subject: [PATCH 2/3] msvc_gendef.pl: Remove unused variable
Commit a5eed4d770674904 swithced to using a hash instead of an array
for the defs, but neglected to remove the unused variable.
---
src/tools/msvc_gendef.pl | 2 --
1 file changed, 2 deletions(-)
diff --git a/src/tools/msvc_gendef.pl b/src/tools/msvc_gendef.pl
index 404076dbbc..97346cc892 100644
--- a/src/tools/msvc_gendef.pl
+++ b/src/tools/msvc_gendef.pl
@@ -6,8 +6,6 @@
use List::Util qw(min);
use Getopt::Long;
-my @def;
-
#
# Script that generates a .DEF file for all objects in a directory
#
--
2.39.2
>From ef4aed0fb3fe7d96662b0b9a5c6b5998916d9d4c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Dagfinn=20Ilmari=20Manns=C3=A5ker?= <ilm...@ilmari.org>
Date: Tue, 2 Jul 2024 13:37:54 +0100
Subject: [PATCH 3/3] pgindent: remove unused variable
Commit b16259b3c1897cf9 removed the --build option, but neglected to
remove the corresponding $build variable.
---
src/tools/pgindent/pgindent | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/tools/pgindent/pgindent b/src/tools/pgindent/pgindent
index 48d83bc434..063ec8ce63 100755
--- a/src/tools/pgindent/pgindent
+++ b/src/tools/pgindent/pgindent
@@ -22,7 +22,7 @@ my $indent_opts =
my $devnull = File::Spec->devnull;
my ($typedefs_file, $typedef_str, @excludes,
- $indent, $build, $diff,
+ $indent, $diff,
$check, $help, @commits,);
$help = 0;
--
2.39.2
