Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign updo EXPR on dir fails but has no error code $! #14841
Comments
From @bulk88Created by @bulk88The docs for "do EXPR" state --------------------------------------------------------------------------- Lets call do() on a dir. --------------------------------------------------------------------------- C:\perl521\srcnewb4opt> C:\perl521\srcnewb4opt> ------------------------------------------------------------------------ So where is the error in $!? http://perl5.git.perl.org/perl.git/blob/HEAD:/t/op/do.t never tests the http://perl5.git.perl.org/perl.git/commitdiff/b2da7ead6803119763b139be37b2cc8c8f522f75 http://perl5.git.perl.org/perl.git/commitdiff/6b845e562be40aac749b544b6d494078c54de4aa I am proposing that do() on a directory set $!/errno to EISDIR instead of 0. I found this bug due to me wanting to remove the -f syscall out of Perl Info |
From gdg@zplane.combulk88 <perlbug-followup@perl.org> [2015-08-09 13:42:44 -0700]:
Would it make sense to somehow combine/consolidate the above perlbug with http://www.nntp.perl.org/group/perl.perl5.porters/2015/05/msg227897.html which is an ongoing debate on the state of the "do EXPR" doc in general? I have no technical skin the game advocating any position, just trying to be |
|
The RT System itself - Status changed from 'new' to 'open' |
From @ikegamiOn Sun, Aug 9, 2015 at 7:04 PM, Glenn Golden <gdg@zplane.com> wrote:
bulk88 is reporting a failure of do() to report an error. This is |
From @ap* Eric Brine <ikegami@adaelis.com> [2015-08-10 05:50]:
The reason the wording of the docs is in contention is that `do` has I had a conversation about it with RJBS that I meant to summarize and Regards, |
From @bulk88Since there were no comments on what the error code should be, I wrote a patch that fixes this bug. See attached. -- |
From @bulk880001-perl-125774-fix-do-dir-returns-no.patchFrom bc453c910fc935db8524e9d6ea19130407c19e9f Mon Sep 17 00:00:00 2001
From: Daniel Dragan <bulk88@hotmail.com>
Date: Sun, 16 Aug 2015 04:30:23 -0400
Subject: [PATCH] [perl #125774] fix do dir returns no $!
do()ing a directory was returning false/empty string in $!, which isn't an
error, yet retval of do, and $@ say $! should have the error code in it.
Fix this by returning EISDIR for dirs, and EINVAL for block devices, since
there is no ENOTFILE, but ENOENT is inappropriate, since the block device
exists. Also linux read() returns EINVAL if the underlying kernel driver
does not implement a read method. Here we say read() (on a high level do())
is not implemented on a directory. Also linux swapon() uses EINVAL for
paths which goto something that exists, but is of wrong type.
Remove "errno = 0" and comment added in b2da7ead68, since now there is no
scenario where errno is uninitialized, since the dir and block device
failure branches now set errno, previously they didnt.
---
pod/perldelta.pod | 6 ++++++
pp_ctl.c | 25 +++++++++++++++++--------
t/op/do.t | 14 +++++++++++++-
3 files changed, 36 insertions(+), 9 deletions(-)
diff --git a/pod/perldelta.pod b/pod/perldelta.pod
index 157d9d1..2ec1f08 100644
--- a/pod/perldelta.pod
+++ b/pod/perldelta.pod
@@ -356,6 +356,12 @@ Perl can again be compiled with any Unicode version. This used to
C<Name_Alias> did not exist prior to Unicode 5.0. L<Unicode::UCD>
incorrectly said it did. This has been fixed.
+=item *
+
+Calling C<do $path> on a directory or block device returned no error code in
+C<$!> even though the retval of C<do> and C<$@> indicated the error is in C<$!>.
+[perl #125774]
+
=back
=head1 Known Problems
diff --git a/pp_ctl.c b/pp_ctl.c
index 9a81583..d80ed77 100644
--- a/pp_ctl.c
+++ b/pp_ctl.c
@@ -3505,15 +3505,22 @@ S_check_type_and_open(pTHX_ SV *name)
errno EACCES, so only do a stat to separate a dir from a real EACCES
caused by user perms */
#ifndef WIN32
- /* we use the value of errno later to see how stat() or open() failed.
- * We don't want it set if the stat succeeded but we still failed,
- * such as if the name exists, but is a directory */
- errno = 0;
-
st_rc = PerlLIO_stat(p, &st);
- if (st_rc < 0 || S_ISDIR(st.st_mode) || S_ISBLK(st.st_mode)) {
+ if (st_rc < 0)
return NULL;
+ else {
+ int eno;
+ if(S_ISBLK(st.st_mode)) {
+ eno = EINVAL;
+ goto not_file;
+ }
+ else if(S_ISDIR(st.st_mode)) {
+ eno = EISDIR;
+ not_file:
+ errno = eno;
+ return NULL;
+ }
}
#endif
@@ -3525,8 +3532,10 @@ S_check_type_and_open(pTHX_ SV *name)
int eno;
st_rc = PerlLIO_stat(p, &st);
if (st_rc >= 0) {
- if(S_ISDIR(st.st_mode) || S_ISBLK(st.st_mode))
- eno = 0;
+ if(S_ISDIR(st.st_mode))
+ eno = EISDIR;
+ else if(S_ISBLK(st.st_mode))
+ eno = EINVAL;
else
eno = EACCES;
errno = eno;
diff --git a/t/op/do.t b/t/op/do.t
index 49c0de3..7eb923d 100644
--- a/t/op/do.t
+++ b/t/op/do.t
@@ -7,6 +7,7 @@ BEGIN {
}
use strict;
no warnings 'void';
+use Errno qw(ENOENT EISDIR);
my $called;
my $result = do{ ++$called; 'value';};
@@ -247,7 +248,7 @@ SKIP: {
my $saved_errno = $!;
ok(!$rv, "do returns false on io errror");
ok(!$saved_error, "\$\@ not set on io error");
- ok($saved_errno, "\$! set on io error");
+ ok($saved_errno == ENOENT, "\$! is ENOENT for nonexistent file");
}
# do subname should not be do "subname"
@@ -280,4 +281,15 @@ SKIP: {
}
}
+# do file $!s must be correct
+{
+ local @INC = ('.'); #want EISDIR not ENOENT
+ my $rv = do 'op'; # /t/op dir
+ my $saved_error = $@;
+ my $saved_errno = $!+0;
+ ok(!$rv, "do dir returns false");
+ ok(!$saved_error, "\$\@ is false on do dir");
+ ok($saved_errno == EISDIR, "\$! is EISDIR on do dir");
+}
+
done_testing();
--
1.7.9.msysgit.0
|
From @bulk88On Sun Aug 16 03:48:41 2015, bulk88 wrote:
Bump. -- |
From @rjbsOn Mon Aug 10 09:12:05 2015, aristotle wrote:
Have you gotten around to that? I don't see it in my folder, so if so: link? -- |
From @rjbsOn Wed Sep 09 06:50:40 2015, bulk88 wrote:
Is there some reason not to apply this? My reading suggests it's a solution to a demonstrable problem. -- |
From zefram@fysh.orgPatch applied as commit d1ac83c. -zefram |
|
@cpansprout - Status changed from 'open' to 'pending release' |
From @khwilliamsonThank you for filing this report. You have helped make Perl better. With the release yesterday of Perl 5.28.0, this and 185 other issues have been Perl 5.28.0 may be downloaded via: If you find that the problem persists, feel free to reopen this ticket. |
|
@khwilliamson - Status changed from 'pending release' to 'resolved' |
Migrated from rt.perl.org#125774 (status was 'resolved')
Searchable as RT125774$