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

stat() warns about unopened filehandle on tied handles #12029

Open
p5pRT opened this issue Mar 31, 2012 · 12 comments
Open

stat() warns about unopened filehandle on tied handles #12029

p5pRT opened this issue Mar 31, 2012 · 12 comments

Comments

@p5pRT
Copy link

p5pRT commented Mar 31, 2012

Migrated from rt.perl.org#112164 (status was 'open')

Searchable as RT112164$

@p5pRT
Copy link
Author

p5pRT commented Mar 31, 2012

From @xdg

Created by @xdg

Calling stat() on a tied handle warns, even if the tied handle has
a defined, non-negative fileno. Example​:

$ perl -MTie​::StdHandle -wE 'tie *FOO, 'Tie​::StdHandle', "/etc/services"; print scalar readline *FOO; say fileno *FOO; stat(*FOO)'
# Network services, Internet style
3
stat() on unopened filehandle FOO at -e line 1, <HANDLE> line 1.

The bug is still in blead.

The same warning used to occur for scalar filehandles, but it
was fixed in the 5.15 development series.

Perl Info

Flags:
    category=core
    severity=medium

Site configuration information for perl 5.14.2:

Configured by david at Thu Jan  5 16:18:53 EST 2012.

Summary of my perl5 (revision 5 version 14 subversion 2) configuration:
   
  Platform:
    osname=linux, osvers=3.0.0-14-generic, archname=x86_64-linux
    uname='linux vulcan 3.0.0-14-generic #23-ubuntu smp mon nov 21 20:28:43 utc 2011 x86_64 x86_64 x86_64 gnulinux '
    config_args='-de -Dprefix=/home/david/perl5/perlbrew/perls/perl-5.14.2'
    hint=recommended, useposix=true, d_sigaction=define
    useithreads=undef, usemultiplicity=undef
    useperlio=define, d_sfio=undef, uselargefiles=define, usesocks=undef
    use64bitint=define, use64bitall=define, uselongdouble=undef
    usemymalloc=n, bincompat5005=undef
  Compiler:
    cc='cc', ccflags ='-fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64',
    optimize='-O2',
    cppflags='-fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include'
    ccversion='', gccversion='4.6.1', gccosandvers=''
    intsize=4, longsize=8, ptrsize=8, doublesize=8, byteorder=12345678
    d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=16
    ivtype='long', ivsize=8, nvtype='double', nvsize=8, Off_t='off_t', lseeksize=8
    alignbytes=8, prototype=define
  Linker and Libraries:
    ld='cc', ldflags =' -fstack-protector -L/usr/local/lib'
    libpth=/usr/local/lib /lib/x86_64-linux-gnu /lib/../lib /usr/lib/x86_64-linux-gnu /usr/lib/../lib /lib /usr/lib
    libs=-lnsl -ldb -ldl -lm -lcrypt -lutil -lc
    perllibs=-lnsl -ldl -lm -lcrypt -lutil -lc
    libc=, so=so, useshrplib=false, libperl=libperl.a
    gnulibc_version='2.13'
  Dynamic Linking:
    dlsrc=dl_dlopen.xs, dlext=so, d_dlsymun=undef, ccdlflags='-Wl,-E'
    cccdlflags='-fPIC', lddlflags='-shared -O2 -L/usr/local/lib -fstack-protector'

Locally applied patches:
    


@INC for perl 5.14.2:
    /home/david/perl5/perlbrew/perls/perl-5.14.2/lib/site_perl/5.14.2/x86_64-linux
    /home/david/perl5/perlbrew/perls/perl-5.14.2/lib/site_perl/5.14.2
    /home/david/perl5/perlbrew/perls/perl-5.14.2/lib/5.14.2/x86_64-linux
    /home/david/perl5/perlbrew/perls/perl-5.14.2/lib/5.14.2
    .


Environment for perl 5.14.2:
    HOME=/home/david
    LANG=en_US.UTF-8
    LANGUAGE (unset)
    LC_COLLATE=C
    LC_CTYPE=en_US.UTF-8
    LC_MESSAGES=en_US.UTF-8
    LD_LIBRARY_PATH (unset)
    LOGDIR (unset)
    PATH=/home/david/perl5/perlbrew/bin:/home/david/perl5/perlbrew/perls/perl-5.14.2/bin:~/bin:~/git/utility-scripts:/opt/perl/current/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games:.
    PERLBREW_BASHRC_VERSION=0.39
    PERLBREW_HOME=/home/david/.perlbrew
    PERLBREW_MANPATH=/home/david/perl5/perlbrew/perls/perl-5.14.2/man
    PERLBREW_PATH=/home/david/perl5/perlbrew/bin:/home/david/perl5/perlbrew/perls/perl-5.14.2/bin
    PERLBREW_PERL=perl-5.14.2
    PERLBREW_ROOT=/home/david/perl5/perlbrew
    PERLBREW_VERSION=0.39
    PERL_BADLANG (unset)
    PERL_EXTUTILS_AUTOINSTALL=--defaultdeps
    SHELL=/bin/bash

@p5pRT
Copy link
Author

p5pRT commented Mar 31, 2012

From @jkeenan

On Sat Mar 31 09​:19​:10 2012, dagolden@​cpan.org wrote​:

This is a bug report for perl from dagolden@​cpan.org,
generated with the help of perlbug 1.39 running under perl 5.14.2.

-----------------------------------------------------------------
[Please describe your issue here]

Calling stat() on a tied handle warns, even if the tied handle has
a defined, non-negative fileno. Example​:

$ perl -MTie​::StdHandle -wE 'tie *FOO, 'Tie​::StdHandle',
"/etc/services"; print scalar readline *FOO; say fileno *FOO;
stat(*FOO)'
# Network services, Internet style
3
stat() on unopened filehandle FOO at -e line 1, <HANDLE> line 1.

The bug is still in blead.

The same warning used to occur for scalar filehandles, but it
was fixed in the 5.15 development series.

That was probably​: https://rt-archive.perl.org/perl5/Ticket/Display.html?id=71002

@p5pRT
Copy link
Author

p5pRT commented Mar 31, 2012

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

@p5pRT
Copy link
Author

p5pRT commented Mar 31, 2012

From @cpansprout

On Sat Mar 31 09​:19​:10 2012, dagolden@​cpan.org wrote​:

Calling stat() on a tied handle warns, even if the tied handle has
a defined, non-negative fileno. Example​:

$ perl -MTie​::StdHandle -wE 'tie *FOO, 'Tie​::StdHandle',
"/etc/services"; print scalar readline *FOO; say fileno *FOO;
stat(*FOO)'
# Network services, Internet style
3
stat() on unopened filehandle FOO at -e line 1, <HANDLE> line 1.

fileno() there is not a real fileno call, but tied(*FOO)->FILENO.

perltie.pod says​:

Tied filehandles are still incomplete. sysopen(), truncate(),
flock(), fcntl(), stat() and -X can't currently be trapped.

So it sounds as though we would need to implement STAT on tied
filehandles before being able to fix this properly (i.e., for all
handle-tie classes).

OTOH, a Tie​::StdHandle object is a globref​:

sub TIEHANDLE
{
my $class = shift;
my $fh = \do { local *HANDLE};
bless $fh,$class;
$fh->OPEN(@​_) if (@​_);
return $fh;
}

so I’m not sure why it’s not already working properly.

--

Father Chrysostomos

@p5pRT
Copy link
Author

p5pRT commented Mar 31, 2012

From @cpansprout

On Sat Mar 31 10​:39​:01 2012, sprout wrote​:

OTOH, a Tie​::StdHandle object is a globref​:

sub TIEHANDLE
{
my $class = shift;
my $fh = \do { local *HANDLE};
bless $fh,$class;
$fh->OPEN(@​_) if (@​_);
return $fh;
}

so I’m not sure why it’s not already working properly.

It’s not already working, because there are *two* globs involved. *FOO
is one glob, which stat() is being called on, and ${tied(*FOO)} is the
other glob, which has a real file handle.

So, again, we would have to make stat(*FOO) call tied(*FOO)->STAT to fix
this.

--

Father Chrysostomos

@p5pRT
Copy link
Author

p5pRT commented Mar 31, 2012

From @xdg

On Sat, Mar 31, 2012 at 7​:39 PM, Father Chrysostomos via RT
<perlbug-followup@​perl.org> wrote​:

fileno() there is not a real fileno call, but tied(*FOO)->FILENO.

Sure. That was just to confirm the file was indeed opened.

Tied filehandles are still incomplete.  sysopen(), truncate(),
flock(), fcntl(), stat() and -X can't currently be trapped.

So it sounds as though we would need to implement STAT on tied
filehandles before being able to fix this properly (i.e., for all
handle-tie classes).

I think that's a good long term goal, but a more immediate fix is that
stat() should return the empty list for tied filehandles. That better
reflects that stat() is failing (because handles don't support STAT),
and avoids giving misleading information (e.g. "unopened") about the
handle.

-- David

@p5pRT
Copy link
Author

p5pRT commented Mar 31, 2012

From @Leont

On Sat, Mar 31, 2012 at 6​:19 PM, David Golden <perlbug-followup@​perl.org> wrote​:

Calling stat() on a tied handle warns, even if the tied handle has
a defined, non-negative fileno.  Example​:

$ perl -MTie​::StdHandle -wE 'tie *FOO, 'Tie​::StdHandle', "/etc/services"; print scalar readline *FOO; say fileno *FOO; stat(*FOO)'
# Network services, Internet style
3
stat() on unopened filehandle FOO at -e line 1, <HANDLE> line 1.

The bug is still in blead.

The same warning used to occur for scalar filehandles, but it
was fixed in the 5.15 development series.

[Please do not change anything below this line]

I think this should do it.

Leon

@p5pRT
Copy link
Author

p5pRT commented Mar 31, 2012

From @Leont

0001-Fix-stat-on-tied-filehandles.patch
From 78bde9f6abd7a1fc3eea157de223db32b294168c Mon Sep 17 00:00:00 2001
From: Leon Timmermans <fawaka@gmail.com>
Date: Sat, 31 Mar 2012 19:28:47 +0200
Subject: [PATCH] Fix stat on tied filehandles

---
 pp_sys.c |   14 ++++++++++++++
 1 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/pp_sys.c b/pp_sys.c
index 49910d2..30b1512 100644
--- a/pp_sys.c
+++ b/pp_sys.c
@@ -2784,6 +2784,7 @@ PP(pp_stat)
                 io = GvIO(gv);
 	    }
             if (io) {
+		    MAGIC* mg;
                     if (IoIFP(io)) {
                         PL_laststatval = 
                             PerlLIO_fstat(PerlIO_fileno(IoIFP(io)), &PL_statcache);   
@@ -2792,6 +2793,19 @@ PP(pp_stat)
                         PL_laststatval =
                             PerlLIO_fstat(my_dirfd(IoDIRP(io)), &PL_statcache);
                         havefp = TRUE;
+		    } else if(SvMAGICAL((SV*)io) && (mg = mg_find((SV*)io, PERL_MAGIC_tiedscalar))) {
+			int fd;
+			ENTER;
+			PUSHMARK(SP);
+			PUSHs(SvTIED_obj(sv, mg));
+			PUTBACK;
+			call_method("FILENO", G_SCALAR);
+			SPAGAIN;
+			fd = POPi;
+			LEAVE;
+			
+			PL_laststatval = PerlLIO_fstat(fd, &PL_statcache);
+>>>>>>> 81c3e5c16a3bf84ffcb41fd9e4a3135c4ad190a9
                     } else {
                         PL_laststatval = -1;
                     }
-- 
1.7.5.4

@p5pRT
Copy link
Author

p5pRT commented Mar 31, 2012

From @cpansprout

On Sat Mar 31 11​:08​:55 2012, LeonT wrote​:

On Sat, Mar 31, 2012 at 6​:19 PM, David Golden <perlbug-
followup@​perl.org> wrote​:

Calling stat() on a tied handle warns, even if the tied handle has
a defined, non-negative fileno. �Example​:

$ perl -MTie​::StdHandle -wE 'tie *FOO, 'Tie​::StdHandle',
"/etc/services"; print scalar readline *FOO; say fileno *FOO;
stat(*FOO)'
# Network services, Internet style
3
stat() on unopened filehandle FOO at -e line 1, <HANDLE> line 1.

The bug is still in blead.

The same warning used to occur for scalar filehandles, but it
was fixed in the 5.15 development series.

[Please do not change anything below this line]

I think this should do it.

Ah, using FILENO to get the file number and then doing a stat with
that--I hadn’t thought of that. But you would also need to do the same
with my_stat_flags in doio.c.

--

Father Chrysostomos

@p5pRT
Copy link
Author

p5pRT commented Mar 31, 2012

From @cpansprout

On Sat Mar 31 10​:30​:46 2012, jkeenan wrote​:

On Sat Mar 31 09​:19​:10 2012, dagolden@​cpan.org wrote​:

This is a bug report for perl from dagolden@​cpan.org,
generated with the help of perlbug 1.39 running under perl 5.14.2.

-----------------------------------------------------------------
[Please describe your issue here]

Calling stat() on a tied handle warns, even if the tied handle has
a defined, non-negative fileno. Example​:

$ perl -MTie​::StdHandle -wE 'tie *FOO, 'Tie​::StdHandle',
"/etc/services"; print scalar readline *FOO; say fileno *FOO;
stat(*FOO)'
# Network services, Internet style
3
stat() on unopened filehandle FOO at -e line 1, <HANDLE> line 1.

The bug is still in blead.

The same warning used to occur for scalar filehandles, but it
was fixed in the 5.15 development series.

That was probably​: https://rt-archive.perl.org/perl5/Ticket/Display.html?id=71002

I don’t think so. That dealt with the special casing that _ gets.

There have been multiple fixes to stat, but I don’t know exactly which
one David Golden is referring to (maybe 8080e3c?). The example above
gives no warning for me in older perls with *FOO changed to $foo and the
tie statement omitted.

--

Father Chrysostomos

@p5pRT
Copy link
Author

p5pRT commented Mar 31, 2012

From @xdg

On Sat, Mar 31, 2012 at 8​:19 PM, Father Chrysostomos via RT
<perlbug-followup@​perl.org> wrote​:

Ah, using FILENO to get the file number and then doing a stat with
that--I hadn’t thought of that.  But you would also need to do the same
with my_stat_flags in doio.c.

As I mentioned to leont in passing at the hackathon, a tied handle
could lie (intentionally or not) about the FILENO and then you'd be
calling stat on the wrong thing.

Short of tied handles doing STAT, I still think best would be to just
have stat return the empty list for tied handles (without a warning).

-- David

@p5pRT
Copy link
Author

p5pRT commented May 17, 2012

From @cpansprout

On Sat Mar 31 10​:55​:15 2012, dagolden@​cpan.org wrote​:

On Sat, Mar 31, 2012 at 7​:39 PM, Father Chrysostomos via RT
<perlbug-followup@​perl.org> wrote​:

fileno() there is not a real fileno call, but tied(*FOO)->FILENO.

Sure. That was just to confirm the file was indeed opened.

Tied filehandles are still incomplete. �sysopen(), truncate(),
flock(), fcntl(), stat() and -X can't currently be trapped.

So it sounds as though we would need to implement STAT on tied
filehandles before being able to fix this properly (i.e., for all
handle-tie classes).

I think that's a good long term goal,

I’d like to get that done for 5.18.

What should the interface be for the STAT call?

Should it be expected to return a 13-element list? Should those
returned values be associated with the _ handle?

What should happen if it returns a list with fewer elements?

Should pp_stat croak, or assume (@​ret, (undef)x(13-@​ret))?

If it croaks, should it be forgiving with ‘return undef’? (Probably not.)

I think I’m in favour of having it croak if @​ret != 13 && @​ret != 0.
And the values should be saved for _.

Filetest calls like -r and -w can be implemented in terms of STAT, but
what do we do about -t -T -B?

but a more immediate fix is that
stat() should return the empty list for tied filehandles. That better
reflects that stat() is failing (because handles don't support STAT),
and avoids giving misleading information (e.g. "unopened") about the
handle.

This can be the fallback for handles without STAT.

--

Father Chrysostomos

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants