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
sysopen() sets $! on success #16064
Comments
From @FGasper=================
^^ It seems $! should only be: - set to 0/q<> on success I observed the same with open() -- I tried filing on it via the perlbug script, but I think I got a “false success”: =================== Please type "yes" if you are [no]: yes Sendmail returned status '1' Thank you for taking the time to file a bug report!Thank you for your time. -FG |
From @LeontOn Sun, Jul 2, 2017 at 5:11 PM, felipe@felipegasper.com <
Setting $! on success is legal, it's value is only defined after a failing I do agree setting it to some error on success is not terribly helpful Leon |
The RT System itself - Status changed from 'new' to 'open' |
From @tonycozOn Sun, 02 Jul 2017 08:27:50 -0700, LeonT wrote:
The attached prevents the error code set that Felipe describes, but as you say, we don't promise that sysopen leaves errno alone on success, so I'm 40% inclined to just reject this ticket. Tony |
From @tonycoz0001-perl-131690-prevent-other-syscalls-modifying-errno-i.patchFrom 89abca172fe8119a11a565899da1f729759b2ca5 Mon Sep 17 00:00:00 2001
From: Tony Cook <tony@develop-help.com>
Date: Wed, 5 Jul 2017 12:01:04 +1000
Subject: (perl #131690) prevent other syscalls modifying errno in sysopen
sysopen() does PerlIO initialization that can modify errno on success,
prevent that initialization from modifying errno.
This doesn't prevent the actual open(3) call from setting errno on
success.
---
perlio.c | 2 ++
t/op/sysio.t | 12 ++++++++++--
2 files changed, 12 insertions(+), 2 deletions(-)
diff --git a/perlio.c b/perlio.c
index 209af1b..5c418d0 100644
--- a/perlio.c
+++ b/perlio.c
@@ -3838,6 +3838,7 @@ PerlIOBuf_pushed(pTHX_ PerlIO *f, const char *mode, SV *arg, PerlIO_funcs *tab)
{
PerlIOBuf *b = PerlIOSelf(f, PerlIOBuf);
const int fd = PerlIO_fileno(f);
+ dSAVE_ERRNO;
if (fd >= 0 && PerlLIO_isatty(fd)) {
PerlIOBase(f)->flags |= PERLIO_F_LINEBUF | PERLIO_F_TTY;
}
@@ -3847,6 +3848,7 @@ PerlIOBuf_pushed(pTHX_ PerlIO *f, const char *mode, SV *arg, PerlIO_funcs *tab)
b->posn = posn;
}
}
+ RESTORE_ERRNO;
return PerlIOBase_pushed(aTHX_ f, mode, arg, tab);
}
diff --git a/t/op/sysio.t b/t/op/sysio.t
index b95def0..5b399fb 100644
--- a/t/op/sysio.t
+++ b/t/op/sysio.t
@@ -6,7 +6,7 @@ BEGIN {
set_up_inc('../lib');
}
-plan tests => 48;
+plan tests => 50;
open(I, 'op/sysio.t') || die "sysio.t: cannot find myself: $!";
@@ -233,7 +233,15 @@ is($@, '');
close(I);
unlink_all $outfile;
-chdir('..');
+SKIP: {
+ skip_if_miniperl("Fcntl requires XS", 2);
+ require Fcntl;
+ $! = 0;
+ ok(sysopen(my $fh, "TEST", Fcntl::O_RDONLY()),
+ "successfully sysopen TEST for read")
+ or skip("sysopen failed", 1);
+ is(0+$!, 0, "sysopen doesn't touch $! on success");
+}
1;
--
2.1.4
|
From @demerphqOn 5 Jul 2017 04:06, "Tony Cook via RT" <perlbug-followup@perl.org> wrote: On Sun, 02 Jul 2017 08:27:50 -0700, LeonT wrote:
The attached prevents the error code set that Felipe describes, but as you Personally I think that this goes against the design of errno. That var is In other words not only is it unsound to expect errno to be unchanged on Yves |
From zefram@fysh.orgTony Cook via RT wrote:
Indeed, we should just reject it. errno, and therefore $!, is generally -zefram |
From cm.perl@abtela.comLe 10/07/2017 à 22:33, Zefram a écrit :
I still do not see why $! should be so tightly anchored to errno. I -Christian |
From zefram@fysh.orgChristian Millour wrote:
That's what it always historically has been, and it's a simple -zefram |
From @tonycozOn Mon, 10 Jul 2017 13:33:47 -0700, zefram@fysh.org wrote:
Based on the discussion and on that in #124232 I'm rejecting this ticket. Tony |
@tonycoz - Status changed from 'open' to 'rejected' |
Migrated from rt.perl.org#131690 (status was 'rejected')
Searchable as RT131690$
The text was updated successfully, but these errors were encountered: