Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[PATCH] perl -e "eof or die" dies #16786

Closed
p5pRT opened this issue Dec 12, 2018 · 9 comments

Comments

@p5pRT
Copy link

commented Dec 12, 2018

Migrated from rt.perl.org#133721 (status was 'resolved')

Searchable as RT133721$

@p5pRT

This comment has been minimized.

Copy link
Author

commented Dec 12, 2018

From @haukex

Hi all,

As the subject says​: These die, when AFAICT they shouldn't​:

$ perl -e "eof or die"
Died at -e line 1.
$ perl -e "eof or die" /dev/null
Died at -e line 1.

These all work​:

$ perl -e "eof() or die" </dev/null
$ perl -e "eof() or die" /dev/null
$ perl -e "<>; eof or die" /dev/null

It appears to be the same behavior on Linux and Windows.

I ran a bisect and the commit that changed this is
32e6532, the change was first released
with 5.12.0 (more specifically, 5.11.0). See also [#60978].

I'm definitely not an expert, but it looks to me like "if (!gv)
RETPUSHNO;" should probably be "if (!gv) RETPUSHYES;" instead? At least
that's what the logic prior to 32e6532 seems to be doing​:
"PUSHs(boolSV(!gv || do_eof(gv)))"

Patch attached. (I didn't see a place where "eof" is really tested in
the test suite?)

Thanks, Regards,
-- Hauke D

@p5pRT

This comment has been minimized.

Copy link
Author

commented Dec 12, 2018

From @haukex

0001-First-eof-should-return-true.patch
From 2d068d8c80002d42cedd3f6f17630a1d70e22086 Mon Sep 17 00:00:00 2001
From: Hauke D <haukex@zero-g.net>
Date: Wed, 12 Dec 2018 22:26:26 +0100
Subject: [PATCH] First "eof" should return true

When no file has previously been opened, "eof" should return true. This
behavior was broken by 32e653230c7ccc (see also [#60978]).
---
 pp_sys.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/pp_sys.c b/pp_sys.c
index 5dc20b14f0..e28e8906f1 100644
--- a/pp_sys.c
+++ b/pp_sys.c
@@ -2121,7 +2121,7 @@ PP(pp_eof)
     }
 
     if (!gv)
-	RETPUSHNO;
+	RETPUSHYES;
 
     if ((io = GvIO(gv)) && (mg = SvTIED_mg((const SV *)io, PERL_MAGIC_tiedscalar))) {
 	return tied_method1(SV_CONST(EOF), SP, MUTABLE_SV(io), mg, newSVuv(which));
-- 
2.19.2

@p5pRT

This comment has been minimized.

Copy link
Author

commented Dec 12, 2018

From @jkeenan

On Wed, 12 Dec 2018 21​:29​:46 GMT, haukex@​zero-g.net wrote​:

Hi all,

As the subject says​: These die, when AFAICT they shouldn't​:

$ perl -e "eof or die"
Died at -e line 1.
$ perl -e "eof or die" /dev/null
Died at -e line 1.

These all work​:

$ perl -e "eof() or die" </dev/null
$ perl -e "eof() or die" /dev/null
$ perl -e "<>; eof or die" /dev/null

It appears to be the same behavior on Linux and Windows.

I ran a bisect and the commit that changed this is
32e6532, the change was first released
with 5.12.0 (more specifically, 5.11.0). See also [#60978].

I'm definitely not an expert, but it looks to me like "if (!gv)
RETPUSHNO;" should probably be "if (!gv) RETPUSHYES;" instead? At least
that's what the logic prior to 32e6532 seems to be doing​:
"PUSHs(boolSV(!gv || do_eof(gv)))"

Patch attached. (I didn't see a place where "eof" is really tested in
the test suite?)

Thanks, Regards,
-- Hauke D

An ack for your consideration​:

#####
$ ack eof t |cat
t/cmd/subval.t​:132​: eof F ? print "not ok $i\n" : print "ok $i\n";
t/cmd/subval.t​:140​: eof F ? print "not ok $i\n" : print "ok $i\n";
t/cmd/subval.t​:141​: &iseof(*F);
t/cmd/subval.t​:145​:sub iseof {
t/cmd/subval.t​:149​: eof UNIQ ? print "(not ok $i)\n" : print "ok $i\n";
t/cmd/subval.t​:159​: eof F ? print "not ok $main'i\n" : print "ok $main'i\n";
t/cmd/subval.t​:167​: eof F ? print "not ok $main'i\n" : print "ok $main'i\n";
t/cmd/subval.t​:168​: &iseof(*F);
t/cmd/subval.t​:171​: sub iseof {
t/cmd/subval.t​:175​: eof UNIQ ? print "not ok $main'i\n" : print "ok $main'i\n";
t/test.pl​:732​: # Hence it's useful to have a way to make STDIN be at eof without
t/comp/require.t​:126​:# interaction with pod (see the eof)
t/run/fresh_perl.t​:96​:system './perl -ne "print if eof" /dev/null'
t/porting/known_pod_issues.dat​:138​:gettimeofday(2)
t/porting/regen.t​:60​: if eof $fh;
t/porting/regen.t​:82​: if (eof $fh) {
t/re/pat_advanced.t​:2372​: # sizeof(STRLEN) != sizeof(UV)
t/uni/readline.t​:24​: is($a .= <Ạ>, 3, '#21628 - $a .= <A> , A eof');
t/op/svleak.t​:315​:# Reification (or lack thereof)
t/op/while.t​:26​:ok(!eof && /vt100/);
t/op/while.t​:36​:ok(eof && !/vt100/ && !$bad);
t/op/while.t​:50​:ok(eof && !$bad);
t/op/while.t​:63​:ok(!eof && /vt100/);
t/op/while.t​:77​:ok(eof && !/vt100/ && !$bad);
t/op/while.t​:95​:ok(eof && !$bad);
t/op/tiehandle.t​:101​:is(eof($fh), '');
t/op/tiehandle.t​:109​:is(eof, 1);
t/op/tiehandle.t​:293​: # make sure the new eof() features work with @​ARGV magic
t/op/tiehandle.t​:304​: is(eof(ARGV), '');
t/op/tiehandle.t​:306​: is(eof(), '');
t/op/tiehandle.t​:309​: is(eof, 1);
t/op/sysio.t​:228​:# eof
t/op/magic.t​:650​:for my $code ('tell $0', 'sysseek $0, 0, 0', 'seek $0, 0, 0', 'eof $0') {
t/op/readline.t​:21​: is($a .= <A>, 3, '#21628 - $a .= <A> , A eof');
t/op/cproto.t​:87​:eof (;*)
t/op/stat.t​:429​: ok(eof FOO, 'at EOF');
t/op/coreamp.t​:878​: 'prototype of &select (or lack thereof)';
t/op/lfs.t​:240​:# eof
t/op/switch.t​:468​: when(eof(DATA)) {
t/op/switch.t​:472​: ok($ok, "eof() not smartmatched");
t/io/perlio.t​:59​:ok(eof($txtfh));;
t/io/perlio.t​:61​:ok(eof($binfh));
t/io/perlio.t​:63​:ok(eof($utffh));
t/io/socket.t​:94​: # child tests are printed once we hit eof
t/io/socket.t​:186​: # child tests are printed once we hit eof
t/io/tell.t​:19​:ok(!eof(TST), "eof is false after open() non-empty file");
t/io/tell.t​:26​: if (eof) {$x++;}
t/io/tell.t​:28​:is($x, 1, "only one eof is in the file");
t/io/tell.t​:32​:ok(eof, "tell() doesn't change current state of eof");
t/io/tell.t​:36​:ok(!eof, "reset at beginning of file clears eof flag");
t/io/tell.t​:44​:ok(!eof($TST), "it doesn't set eof flag");
t/io/tell.t​:52​:ok(eof, "it sets eof flag");
t/io/tell.t​:155​:# be at eof after opening a file but before seeking, reading, or writing.
t/io/tell.t​:164​: fail "# TODO​: file pointer not at eof";
t/io/tell.t​:167​: fail "# TODO​: Hit bug posix-2056. file pointer not at eof";
t/io/tell.t​:170​: fail "file pointer not at eof";
t/io/tell.t​:182​:eof $fh;
t/io/tell.t​:183​:is(tell, 0, "argless tell after eof \$coercible");
t/io/tell.t​:184​:eof *$fh;
t/io/tell.t​:185​:is(tell, 0, "argless tell after eof *\$coercible");
t/io/argv.t​:82​: runperl( prog => 'eof()', stdin => "nothing\n" );
t/io/argv.t​:83​: is( 0+$?, 0, q(eof() doesn't segfault) );
t/io/argv.t​:90​: if (eof()) {
t/io/argv.t​:119​:ok( eof TRY );
t/io/argv.t​:123​: ok( eof NEVEROPENED, 'eof() true on unopened filehandle' );
t/io/argv.t​:128​:ok( !eof(), 'STDIN has something' );
t/io/argv.t​:136​: ok( eof(), 'eof() true with empty @​ARGV' );
t/io/argv.t​:139​: ok( !eof() );
t/io/argv.t​:142​: ok( !eof() );
t/io/argv.t​:145​: ok( eof(), 'eof() true after closing ARGV' );
t/io/argv.t​:237​: like($x, qr/^Can't open echo foo \|​: .* at -e line 1, <> line 3/, '<<>> does not treat ...| as fork after eof');
t/io/argv.t​:241​:fresh_perl_is( <<'**PROG**', "foobar", {}, "ARGV aliasing and eof()" );
t/io/argv.t​:249​: print "bar" if eof();
t/lib/warnings/doio​:45​: Filehandle %s opened only for output [Perl_do_eof]
t/lib/warnings/doio​:46​: my $a = eof STDOUT
t/lib/warnings/doio​:267​:# doio.c [Perl_do_eof]
t/lib/warnings/doio​:269​:my $a = eof STDOUT ;
t/lib/warnings/doio​:271​:$a = eof STDOUT ;
t/lib/warnings/op​:411​:eof STDIN ; # OP_EOF
t/lib/warnings/op​:460​:Useless use of eof in void context at - line 44.
t/lib/warnings/op​:530​:eof STDIN ; # OP_EOF
#####

Since eof() is often used to answer the question, "Is there any more input needing processing?", I would suggest tests should be written under t/io/.

Thank you very much.

--
James E Keenan (jkeenan@​cpan.org)

@p5pRT

This comment has been minimized.

Copy link
Author

commented Dec 12, 2018

The RT System itself - Status changed from 'new' to 'open'

@p5pRT

This comment has been minimized.

Copy link
Author

commented Jan 2, 2019

From @tonycoz

On Wed, 12 Dec 2018 13​:29​:46 -0800, haukex@​zero-g.net wrote​:

Patch attached. (I didn't see a place where "eof" is really tested in
the test suite?)

Yeah, eof testing is kind of messy.

I've added a TODO test in b4e880f, applied your change as eb699a9 with the addition is disarming the TODO.

Thanks,
Tony

@p5pRT

This comment has been minimized.

Copy link
Author

commented Jan 2, 2019

@tonycoz - Status changed from 'open' to 'pending release'

@p5pRT

This comment has been minimized.

Copy link
Author

commented Jan 3, 2019

From @haukex

Hi,

Thank you very much!

and Happy New Year to all :-)

Regards,
-- Hauke D

On 02.01.19 02​:16, Tony Cook via RT wrote​:

On Wed, 12 Dec 2018 13​:29​:46 -0800, haukex@​zero-g.net wrote​:

Patch attached. (I didn't see a place where "eof" is really tested in
the test suite?)

Yeah, eof testing is kind of messy.

I've added a TODO test in b4e880f, applied your change as eb699a9 with the addition is disarming the TODO.

Thanks,
Tony

@p5pRT

This comment has been minimized.

Copy link
Author

commented May 22, 2019

From @khwilliamson

Thank you for filing this report. You have helped make Perl better.

With the release today of Perl 5.30.0, this and 160 other issues have been
resolved.

Perl 5.30.0 may be downloaded via​:
https://metacpan.org/release/XSAWYERX/perl-5.30.0

If you find that the problem persists, feel free to reopen this ticket.

@p5pRT

This comment has been minimized.

Copy link
Author

commented May 22, 2019

@khwilliamson - Status changed from 'pending release' to 'resolved'

@p5pRT p5pRT closed this May 22, 2019
@p5pRT p5pRT added the Severity Low label Oct 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
1 participant
You can’t perform that action at this time.