Re: PostgresNode::_update_pid using undefined variables in tap tests

On Tue, Dec 8, 2015 at 6:09 PM, Michael Paquier
<michael(dot)paquier(at)gmail(dot)com> wrote:
> On Wed, Dec 9, 2015 at 4:47 AM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>> On Thu, Dec 3, 2015 at 11:28 PM, Michael Paquier
>> <michael(dot)paquier(at)gmail(dot)com> wrote:
>> > This does not impact the run, but it creates unwelcome warnings in the
>> > logs. This is actually caused by the following code in PostgresNode
>> > that uses an incorrect check to see if the file has been correctly
>> > opened or not:
>> > open my $pidfile, $self->data_dir . "/postmaster.pid";
>> > if (not defined $pidfile)
>> >
>> > One way to fix this is to use if(open(...)), a second way I know of is
>> > to check if the opened file handle matches tell($pidfile) == -1. The
>> > patch attached uses the first method to fix the issue.
>>
>> My Perl-fu must be getting weak. What's wrong with the existing code?
>
> This code should have checked for the return result of open instead of
> looking at $pidfile. This has been noticed by Noah as well afterwards
> and already addressed as 9821492.

I see that open() returns a value, but I figured $pidfile would end up
as undef if open failed. I see that's not the case:

[rhaas pgsql]$ perl -MData::Dumper -e 'open(my $pidfile, "<",
"/fscsfasf") || warn $!; print Dumper($pidfile);'
No such file or directory at -e line 1.
$VAR1 = \*{'::$pidfile'};

Boy, that's awful. Whoever designed that bit of wonderfulness should
have their language design license revoked.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company