Undefined value returned on call to readline (<>)?

[Intrepid]

Dear Monks, it's good to know we can still reach each other. Perlmonks was completely offline a couple days ago. I felt like someone had stabbed me in the shoulder blade.

So what I am doing this cold winter's eve is playing Mr. Helpful with someone else's code, again. Tonight, it's Proc-PID-File 1.29. In running the test suite I tripped several instances of the warning Use of uninitialized value in pattern match (m//) at blib/lib/Proc/PID/File.pm line 286. I will in the interests of convenience to the reader, show the subroutine (with inserted comment by me) below:

# Returns the process id currently stored in the file set.  If the met
+hod
# is passed a file handle, it will return the value, leaving the file 
+handle
# locked.  This is useful for atomic operations where the caller needs
+ to
# write to the file after the read without allowing other dirty writes
+.
# 
# Please note, when passing a file handle, caller is responsible for
# closing it. Also, file handles must be passed by reference!

sub read {
    my ($self, $fh) = @_;

    sysopen($fh, $self->{path}, O_RDWR|O_CREAT)
        || die qq/Cannot open pid file "$self->{path}": $!\n/;
    flock($fh, LOCK_EX | LOCK_NB)
        || die qq/pid "$self->{path}" already locked: $!\n/;
# WARNING: Use of uninitialized value in pattern match (m//):
    my ($pid) = <$fh> =~ /^(\d+)/;
    close $fh if @_ == 1;

    $self->debug("read(\"$self->{path}\") = " . ($pid || ""));
    return $pid;
    }

Is readline really returning undefined? If so, under what conditions (aside from an empty file) would readline return undef? If it would do no harm, we could sort it by saying no warnings "undefined"; in the scope of the subroutine. Any observations, good monks?

Feb 17, 2025 at 03:30 UTC

[ikegami]

EOF or error. Read errors tend to be program errors (e.g. using a closed or unopened handle) rather than actual I/O errors.

[Anonymous Monk]

FYI, the author isn't interested in fixing it: https://rt.cpan.org/Public/Bug/Display.html?id=108434

[choroba]

The fix presented in the bug report is not nice. It uses the my $var if ..., discussed for example in Conditional initialization of my-variables.

It would've been cleaner to use something like

                || die qq/Cannot open pid file "$self->{path}": $!\n/;
        flock($fh, LOCK_EX | LOCK_NB)
         || die qq/pid "$self->{path}" already locked: $!\n/;
-       my ($pid) = <$fh> =~ /^(\d+)/;
+       my $maybe = <$fh>;
+       $maybe = "" unless defined $maybe;
+       my ($pid) = $maybe =~ /^(\d+)/;
        close $fh if @_ == 1;

[Intrepid]

"It would've been cleaner to use something like"
...


I happily accept your patch, choroba - if only I was the module author. And I read the bug thread over at Debian and I am in agreement that it was not nice.

This was interesting, educational for me, points for choroba and ikegami. The reply from ikegami confirmed what I suspected, and the discussion at Conditional initialization of my-variables was enlightening. I had looked at that code in Proc-PID-File and it bothered me with out me knowing exactly why. I guess I have some good Perl instincts after all; sometimes we code too compactly, saving a couple more lines of code but introducing a problem in doing so. Yes, indeed, Perl lends itself to obfuscation.

Feb 18, 2025 at 19:51 UTC

[NERDVANA]

In case you're open to other ideas, I suggest getting rid of pid files entirely and using a service supervisor like s6 or Runit.
• Benefits of Runit design
• Features of s6 vs. other options