From: Tom Lane <tgl@sss.pgh.pa.us>
To: Andres Freund <andres@anarazel.de>
cc: pgsql-committers@lists.postgresql.org,
        Andrew Dunstan <andrew@dunslane.net>
Subject: Re: pgsql: ecpg: Output dir, source dir,
 stamp file argument for preproc/*.
In-reply-to: <20220718215042.sl3hivoupdb7lkwv@awork3.anarazel.de>
References: <E1oDWqn-000ADD-Oh@gemulon.postgresql.org>
 <20220718211408.o2uyu5kvud56ihlj@awork3.anarazel.de>
 <822625.1658179641@sss.pgh.pa.us>
 <20220718215042.sl3hivoupdb7lkwv@awork3.anarazel.de>
Comments: In-reply-to Andres Freund <andres@anarazel.de>
	message dated "Mon, 18 Jul 2022 14:50:42 -0700"
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="----- =_aaaaaaaaaa0"
Content-ID: <837727.1658181932.0@sss.pgh.pa.us>
Date: Mon, 18 Jul 2022 18:06:22 -0400
Message-ID: <838180.1658181982@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/838180.1658181982%40sss.pgh.pa.us>
Precedence: bulk

------- =_aaaaaaaaaa0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <837727.1658181932.1@sss.pgh.pa.us>

Andres Freund <andres@anarazel.de> writes:
> That'd make some sense - but it doesn't look like perlcritic is digging that
> deep.

Meh, that's the other explanation :-(

> I'll push the obvious fix in a bit.

Cool.  While looking at this, I wondered if parse.pl's handy-dandy
little exception

no warnings 'uninitialized';

might be related to the issue.  Evidently it's not, but now that
I've noticed it I definitely think it's not up to project standard.
I took it out and fixed the half-dozen places where I got warnings,
as attached.  Seem like a good fix?

			regards, tom lane


------- =_aaaaaaaaaa0
Content-Type: text/x-diff;
	name="avoid-uninitialized-variables-in-parse.pl.patch";
	charset="us-ascii"
Content-ID: <837727.1658181932.2@sss.pgh.pa.us>
Content-Description: avoid-uninitialized-variables-in-parse.pl.patch
Content-Transfer-Encoding: quoted-printable

diff --git a/src/interfaces/ecpg/preproc/parse.pl b/src/interfaces/ecpg/pr=
eproc/parse.pl
index a15f563ad4..38548f24e6 100644
--- a/src/interfaces/ecpg/preproc/parse.pl
+++ b/src/interfaces/ecpg/preproc/parse.pl
@@ -14,7 +14,6 @@
 =

 use strict;
 use warnings;
-no warnings 'uninitialized';
 use Getopt::Long;
 =

 my $srcdir  =3D '.';
@@ -40,7 +39,8 @@ my $tokenmode             =3D 0;
 =

 my (%buff, $infield, $comment, %tokens, %addons);
 my ($stmt_mode, @fields);
-my ($line,      $non_term_id);
+my $line =3D '';
+my $non_term_id;
 =

 =

 # some token have to be replaced by other symbols
@@ -195,6 +195,16 @@ sub main
 		# Now split the line into individual fields
 		my @arr =3D split(' ');
 =

+		if (!@arr)
+		{
+			# empty line: in tokenmode 1, emit an empty line, else ignore
+			if ($tokenmode =3D=3D 1)
+			{
+				add_to_buffer('orig_tokens', '');
+			}
+			next line;
+		}
+
 		if ($arr[0] eq '%token' && $tokenmode =3D=3D 0)
 		{
 			$tokenmode =3D 1;
@@ -341,7 +351,8 @@ sub main
 =

 			# Are we looking at a declaration of a non-terminal ?
 			if (($arr[$fieldIndexer] =3D~ /[A-Za-z0-9]+:/)
-				|| $arr[ $fieldIndexer + 1 ] eq ':')
+				|| (   $fieldIndexer + 1 < scalar(@arr)
+					&& $arr[ $fieldIndexer + 1 ] eq ':'))
 			{
 				$non_term_id =3D $arr[$fieldIndexer];
 				$non_term_id =3D~ tr/://d;
@@ -409,11 +420,13 @@ sub main
 			if (   $copymode
 				&& !$prec
 				&& !$comment
+				&& $fieldIndexer < scalar(@arr)
 				&& length($arr[$fieldIndexer])
 				&& $infield)
 			{
 				if ($arr[$fieldIndexer] ne 'Op'
-					&& (   $tokens{ $arr[$fieldIndexer] } > 0
+					&& ((   defined $tokens{ $arr[$fieldIndexer] }
+							&& $tokens{ $arr[$fieldIndexer] } > 0)
 						|| $arr[$fieldIndexer] =3D~ /'.+'/)
 					|| $stmt_mode =3D=3D 1)
 				{
@@ -472,11 +485,12 @@ sub include_addon
 	my $rec =3D $addons{$block};
 	return 0 unless $rec;
 =

-	if ($rec->{type} eq 'rule')
+	my $rectype =3D (defined $rec->{type}) ? $rec->{type} : '';
+	if ($rectype eq 'rule')
 	{
 		dump_fields($stmt_mode, $fields, ' { ');
 	}
-	elsif ($rec->{type} eq 'addon')
+	elsif ($rectype eq 'addon')
 	{
 		add_to_buffer('rules', ' { ');
 	}
@@ -487,7 +501,7 @@ sub include_addon
 =

 	push(@{ $buff{$buffer} }, @{ $rec->{lines} });
 =

-	if ($rec->{type} eq 'addon')
+	if ($rectype eq 'addon')
 	{
 		dump_fields($stmt_mode, $fields, '');
 	}

------- =_aaaaaaaaaa0--