Skip Menu |
Report information
Id: 131690
Status: rejected
Priority: 0/
Queue: perl5

Owner: Nobody
Requestors: felipe [at] felipegasper.com
Cc:
AdminCc:

Operating System: (no value)
PatchStatus: (no value)
Severity: low
Type: unknown
Perl Version: (no value)
Fixed In: (no value)

Attachments
0001-perl-131690-prevent-other-syscalls-modifying-errno-i.patch



To: perlbug [...] perl.org
Subject: sysopen() sets $! on success
From: Felipe Gasper <felipe [...] felipegasper.com>
Date: Sun, 2 Jul 2017 10:07:52 -0400
Download (untitled) / with headers
text/plain 710b
================= Show quoted text
> perl -Mstrict -E'$! = 0; sysopen my $fh, "foo", 0 or die 123; say $! || "no error"'
Inappropriate ioctl for device ================= ^^ It seems $! should only be: - set to 0/q<> on success - a useful value on failure I observed the same with open() -- I tried filing on it via the perlbug script, but I think I got a “false success”: =================== Are you certain you want to send this message? Please type "yes" if you are [no]: yes unknown: fatal: file /etc/postfix/main.cf: parameter mail_owner: user postfix has same user ID as _postfix Sendmail returned status '1' Thank you for taking the time to file a bug report! =================== Thank you for your time. -FG
Date: Sun, 2 Jul 2017 17:27:17 +0200
From: Leon Timmermans <fawaka [...] gmail.com>
CC: "bugs-bitbucket [...] rt.perl.org" <bugs-bitbucket [...] rt.perl.org>
Subject: Re: [perl #131690] sysopen() sets $! on success
To: Perl5 Porters <perl5-porters [...] perl.org>
Download (untitled) / with headers
text/plain 834b
On Sun, Jul 2, 2017 at 5:11 PM, felipe@felipegasper.com <perlbug-followup@perl.org> wrote:
Show quoted text
=================
> perl -Mstrict -E'$! = 0; sysopen my $fh, "foo", 0 or die 123; say $! || "no error"'
Inappropriate ioctl for device
=================

^^ It seems $! should only be:

- set to 0/q<> on success
- a useful value on failure

I observed the same with open() -- I tried filing on it via the perlbug script, but I think I got a “false success”:

Setting $! on success is legal, it's value is only defined after a failing call. The one thing POSIX doesn't allow for any of its functions though is setting it to zero, and I do believe we should be following that.

I do agree setting it to some error on success is not terribly helpful though, adding a few localizations could reduce confusion.

Leon
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 603b
On Sun, 02 Jul 2017 08:27:50 -0700, LeonT wrote: Show quoted text
> Setting $! on success is legal, it's value is only defined after a failing > call. The one thing POSIX doesn't allow for any of its functions though is > setting it to zero, and I do believe we should be following that. > > I do agree setting it to some error on success is not terribly helpful > though, adding a few localizations could reduce confusion.
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
Subject: 0001-perl-131690-prevent-other-syscalls-modifying-errno-i.patch
From 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
Date: Wed, 5 Jul 2017 10:38:56 +0200
From: demerphq <demerphq [...] gmail.com>
To: Perl RT Bug Tracker <perlbug-followup [...] perl.org>
Subject: Re: [perl #131690] sysopen() sets $! on success
Download (untitled) / with headers
text/plain 1.4k


On 5 Jul 2017 04:06, "Tony Cook via RT" <perlbug-followup@perl.org> wrote:
Show quoted text
On Sun, 02 Jul 2017 08:27:50 -0700, LeonT wrote:
> Setting $! on success is legal, it's value is only defined after a failing
> call. The one thing POSIX doesn't allow for any of its functions though is
> setting it to zero, and I do believe we should be following that.
>
> I do agree setting it to some error on success is not terribly helpful
> though, adding a few localizations could reduce confusion.

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.rn

Personally I think that this goes against the design of errno. That var is relevant if and only if a system call says so and only immediately after it says so. Imo it is totally possible and reasonable for a system call to internally call other system calls which may set errno elegitimately which is then gracefully handled allowing the outer system call to return success, at which point errno has legitimately changed.

In other words not only is it unsound to expect errno to be unchanged on success people should assume that it *will* change on success. 
IMO  the only time we should worry about the volatility of errno is when an invisible internal operation might clobber it before the user has had a chance to copy it after a user level operation. 

Yves
Show quoted text
Date: Mon, 10 Jul 2017 21:33:29 +0100
From: Zefram <zefram [...] fysh.org>
To: perl5-porters [...] perl.org
Subject: Re: [perl #131690] sysopen() sets $! on success
Download (untitled) / with headers
text/plain 308b
Tony Cook via RT wrote: Show quoted text
>we don't promise that sysopen leaves errno alone on success, so I'm 40% >inclined to just reject this ticket.
Indeed, we should just reject it. errno, and therefore $!, is generally allowed to be clobbered on success. Changing that in just one place doesn't seem useful. -zefram
Date: Mon, 10 Jul 2017 23:54:10 +0200
From: Christian Millour <cm.perl [...] abtela.com>
To: perl5-porters [...] perl.org
Subject: Re: [perl #131690] sysopen() sets $! on success
Download (untitled) / with headers
text/plain 418b
Le 10/07/2017 à 22:33, Zefram a écrit : Show quoted text
> errno, and therefore $!, is generally > allowed to be clobbered on success. Changing that in just one place > doesn't seem useful. > > -zefram >
I still do not see why $! should be so tightly anchored to errno. I would appreciate polite perl operators (see https://rt.perl.org/Public/Bug/Display.html?id=124232#txn-1340682). -Christian
From: Zefram <zefram [...] fysh.org>
Date: Mon, 10 Jul 2017 23:58:36 +0100
To: perl5-porters [...] perl.org
Subject: Re: [perl #131690] sysopen() sets $! on success
Download (untitled) / with headers
text/plain 436b
Christian Millour wrote: Show quoted text
>I still do not see why $! should be so tightly anchored to errno.
That's what it always historically has been, and it's a simple implementation. To get cleaner semantics would mean a bunch of implementation and maintenance work, which wouldn't be worthwhile merely for a slightly nicer $!. If we wanted to design a cleaner system, we'd turn the syscall errors into Perl exceptions (hello autodie). -zefram
RT-Send-CC: perl5-porters [...] perl.org
Download (untitled) / with headers
text/plain 452b
On Mon, 10 Jul 2017 13:33:47 -0700, zefram@fysh.org wrote: Show quoted text
> Tony Cook via RT wrote:
> >we don't promise that sysopen leaves errno alone on success, so I'm 40% > >inclined to just reject this ticket.
> > Indeed, we should just reject it. errno, and therefore $!, is generally > allowed to be clobbered on success. Changing that in just one place > doesn't seem useful.
Based on the discussion and on that in #124232 I'm rejecting this ticket. Tony


This service is sponsored and maintained by Best Practical Solutions and runs on Perl.org infrastructure.

For issues related to this RT instance (aka "perlbug"), please contact perlbug-admin at perl.org