search

Perl / perl5

🐪 The Perl programming language
https://dev.perl.org/perl5/
Other
1.93k stars 551 forks source link

VMS::Filespec and PVLV globs #10586

Closed p5pRT closed 13 years ago

p5pRT commented 14 years ago

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

Searchable as RT77500$

p5pRT commented 14 years ago

From @cpansprout

There are three instances of ‘SvTYPE(mysv) == SVt_PVGV’ in vms/vms.c\, one in candelete_fromperl and two in rmscopy_from_perl. I think they should actually be isGV_with_GP(mysv).

Without that change\, a glob in a PVLV will not be recognised as a glob. Currently\, a PVLV will never hold a glob\, though that was supposed to be possible as of 5.9.1. Half the perl source code was modified to support them. See bug #77362.

Once the patch for #77632 is applied\, it should be possible to write failing tests for this.

To test these you can put a glob in a PVLV by passing a nonexistent hash element to a subroutine and then assigning a glob to it​:

  my %hash;   sub { for(shift) {   $_ = *foo;   # ... test candelete and rmscopy with $_ ...   }}->($hash{any_key});

Not having access to VMS\, I am hesitant to write a patch myself\, as I have no way of testing it.

This perlbug -d output is quite meaningless\, as it’s not for the machine on which this bug will occur​:

Flags​:   category=core   severity=low

Site configuration information for perl 5.13.4​:

Configured by sprout at Fri Aug 20 23​:24​:53 PDT 2010.

Summary of my perl5 (revision 5 version 13 subversion 4 patch v5.13.4-16-g16c9153) configuration​:   Snapshot of​: 9b47cddefd9b4a322e6382c8979ceeb2c3ac25c9   Platform​:   osname=darwin\, osvers=10.4.0\, archname=darwin-2level   uname='darwin pint.local 10.4.0 darwin kernel version 10.4.0​: fri apr 23 18​:28​:53 pdt 2010; root​:xnu-1504.7.4~1release_i386 i386 '   config_args='-de -Dusedevel'   hint=recommended\, useposix=true\, d_sigaction=define   useithreads=undef\, usemultiplicity=undef   useperlio=define\, d_sfio=undef\, uselargefiles=define\, usesocks=undef   use64bitint=undef\, use64bitall=undef\, uselongdouble=undef   usemymalloc=n\, bincompat5005=undef   Compiler​:   cc='cc'\, ccflags ='-fno-common -DPERL_DARWIN -no-cpp-precomp -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include'\,   optimize='-O3'\,   cppflags='-no-cpp-precomp -fno-common -DPERL_DARWIN -no-cpp-precomp -fno-strict-aliasing -pipe -fstack-protector -I/usr/local/include'   ccversion=''\, gccversion='4.2.1 (Apple Inc. build 5664)'\, gccosandvers=''   intsize=4\, longsize=4\, ptrsize=4\, doublesize=8\, byteorder=1234   d_longlong=define\, longlongsize=8\, d_longdbl=define\, longdblsize=16   ivtype='long'\, ivsize=4\, nvtype='double'\, nvsize=8\, Off_t='off_t'\, lseeksize=8   alignbytes=8\, prototype=define   Linker and Libraries​:   ld='env MACOSX_DEPLOYMENT_TARGET=10.3 cc'\, ldflags =' -fstack-protector -L/usr/local/lib'   libpth=/usr/local/lib /usr/lib   libs=-ldbm -ldl -lm -lutil -lc   perllibs=-ldl -lm -lutil -lc   libc=/usr/lib/libc.dylib\, so=dylib\, useshrplib=false\, libperl=libperl.a   gnulibc_version=''   Dynamic Linking​:   dlsrc=dl_dlopen.xs\, dlext=bundle\, d_dlsymun=undef\, ccdlflags=' '   cccdlflags=' '\, lddlflags=' -bundle -undefined dynamic_lookup -L/usr/local/lib -fstack-protector'

Locally applied patches​:  

@​INC for perl 5.13.4​:   /usr/local/lib/perl5/site_perl/5.13.4/darwin-2level   /usr/local/lib/perl5/site_perl/5.13.4   /usr/local/lib/perl5/5.13.4/darwin-2level   /usr/local/lib/perl5/5.13.4   /usr/local/lib/perl5/site_perl   .

Environment for perl 5.13.4​:   DYLD_LIBRARY_PATH (unset)   HOME=/Users/sprout   LANG=en_US.UTF-8   LANGUAGE (unset)   LD_LIBRARY_PATH (unset)   LOGDIR (unset)   PATH=/usr/bin​:/bin​:/usr/sbin​:/sbin​:/usr/local/bin​:/usr/X11/bin​:/usr/local/bin   PERL_BADLANG (unset)   SHELL=/bin/bash

p5pRT commented 14 years ago

From @craigberry

On Sun\, Aug 29\, 2010 at 4​:12 PM\, Father Chrysostomos \perlbug\-followup@​perl\.org wrote​:

# New Ticket Created by  Father Chrysostomos # Please include the string​:  [perl #77500] # in the subject line of all future correspondence about this issue. # \<URL​: http​://rt.perl.org/rt3/Ticket/Display.html?id=77500 >

There are three instances of ‘SvTYPE(mysv) == SVt_PVGV’ in vms/vms.c\, one in candelete_fromperl and two in rmscopy_from_perl. I think they should actually be isGV_with_GP(mysv).

Without that change\, a glob in a PVLV will not be recognised as a glob. Currently\, a PVLV will never hold a glob\, though that was supposed to be possible as of 5.9.1. Half the perl source code was modified to support them. See bug #77362.

Once the patch for #77632 is applied\, it should be possible to write failing tests for this.

To test these you can put a glob in a PVLV by passing a nonexistent hash element to a subroutine and then assigning a glob to it​:

 my %hash;  sub { for(shift) {   $_ = *foo;   # ... test candelete and rmscopy with $_ ...  }}->($hash{any_key});

Not having access to VMS\, I am hesitant to write a patch myself\, as I have no way of testing it.

Thanks for spotting this. The code in question has been there for fourteen-plus years (since 5.002) so it's unsurprising what it does is no longer up to snuff. I couldn't find any documentation on isGV_with_GP\, but it apparently came on the scene about four years ago​:

http​://perl5.git.perl.org/perl.git/blobdiff/fb4fc1fa

and your suggestion sounds similar to what people have been doing in other places since then. I'm testing it now and should be able to report back within a couple of days.

p5pRT commented 14 years ago

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

p5pRT commented 14 years ago

From @craigberry

On Sun\, Aug 29\, 2010 at 10​:17 PM\, Craig A. Berry \craig\.a\.berry@&#8203;gmail\.com wrote​:

On Sun\, Aug 29\, 2010 at 4​:12 PM\, Father Chrysostomos \perlbug\-followup@&#8203;perl\.org wrote​:

# New Ticket Created by  Father Chrysostomos # Please include the string​:  [perl #77500] # in the subject line of all future correspondence about this issue. # \<URL​: http​://rt.perl.org/rt3/Ticket/Display.html?id=77500 >

There are three instances of ‘SvTYPE(mysv) == SVt_PVGV’ in vms/vms.c\, one in candelete_fromperl and two in rmscopy_from_perl. I think they should actually be isGV_with_GP(mysv).

Without that change\, a glob in a PVLV will not be recognised as a glob. Currently\, a PVLV will never hold a glob\, though that was supposed to be possible as of 5.9.1. Half the perl source code was modified to support them. See bug #77362.

Done in 6d24fbd10b671f02fd7526bee66bd955389d9ee4. I see no new test failures.

Once the patch for #77632 is applied\, it should be possible to write failing tests for this.

To test these you can put a glob in a PVLV by passing a nonexistent hash element to a subroutine and then assigning a glob to it​:

 my %hash;  sub { for(shift) {   $_ = *foo;   # ... test candelete and rmscopy with $_ ...  }}->($hash{any_key});

Not having access to VMS\, I am hesitant to write a patch myself\, as I have no way of testing it.

I have not added any tests as I don't see quite what I can hang my hat on from Perl that's a reliable success or failure condition. I did experiment with the following test program​:

$ type foo.pl use strict; my %hash;

sub some_sub {   for(shift) {   $_ = *foo;   my $x = VMS​::Filespec​::candelete($_);   print $! unless $x;   } }

some_sub($hash{'nonexistent_key'}); __END__;

Before the change\, that gives me​:

$ perl foo.pl no such file or directory

after the change​:

$ perl foo.pl invalid argument

Stepping through in the VMS debugger (the gdb equivalent)\, I've verified that after the change this passes the glob test and proceeds to test the SV with GvIOp (a test it fails\, yielding EINVAL)\, whereas before it didn't recognize it as glob and proceeded to treat the input argument as a string containing a filename.

So I think it's working and the ticket can be closed\, but there's more than one condition that sets EINVAL\, so if there is a reliable way to make sure it stays working I'm not seeing it.

p5pRT commented 14 years ago

From @cpansprout

On Oct 3\, 2010\, at 5​:15 PM\, Craig Berry via RT wrote​:

So I think it's working and the ticket can be closed\, but there's more than one condition that sets EINVAL\, so if there is a reliable way to make sure it stays working I'm not seeing it.

What happens if you create a temporary file\, open it\, and pass the handle to candelete?

p5pRT commented 14 years ago

From @craigberry

On Sun\, Oct 3\, 2010 at 8​:12 PM\, Father Chrysostomos \sprout@&#8203;cpan\.org wrote​:

On Oct 3\, 2010\, at 5​:15 PM\, Craig Berry via RT wrote​:

So I think it's working and the ticket can be closed\, but there's more than one condition that sets EINVAL\, so if there is a reliable way to make sure it stays working I'm not seeing it.

What happens if you create a temporary file\, open it\, and pass the handle to candelete?

That still works.

p5pRT commented 14 years ago

From @cpansprout

On Oct 3\, 2010\, at 7​:44 PM\, Craig Berry via RT wrote​:

On Sun\, Oct 3\, 2010 at 8​:12 PM\, Father Chrysostomos \sprout@&#8203;cpan\.org wrote​:

On Oct 3\, 2010\, at 5​:15 PM\, Craig Berry via RT wrote​:

So I think it's working and the ticket can be closed\, but there's more than one condition that sets EINVAL\, so if there is a reliable way to make sure it stays working I'm not seeing it.

What happens if you create a temporary file\, open it\, and pass the handle to candelete?

That still works.

Then can you assign that handle to a PVLV and use it for testing?

p5pRT commented 13 years ago

@cpansprout - Status changed from 'open' to 'resolved'