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

filename - handled incorrectly with inplace editing #13634

Open
p5pRT opened this issue Mar 2, 2014 · 2 comments
Open

filename - handled incorrectly with inplace editing #13634

p5pRT opened this issue Mar 2, 2014 · 2 comments

Comments

@p5pRT
Copy link

@p5pRT p5pRT commented Mar 2, 2014

Migrated from rt.perl.org#121358 (status was 'new')

Searchable as RT121358$

@p5pRT
Copy link
Author

@p5pRT p5pRT commented Mar 2, 2014

From @nwc10

Created by @nwc10

The filename - is not handled correctly by in place editing. The semantics
of in place editing were changed as part of this commit between perl-5.004
and perl-5.005​:

commit 85aff57
Author​: Chip Salzenberg <chip@​pobox.com>
Date​: Tue Feb 3 04​:16​:50 1998 -0500

  Some Chip patches (some tweaked to match _5x source)​:
  Subject​: [PATCH] local leakage
  Date​: Tue, 3 Feb 1998 09​:16​:50 -0500 (EST)
  Subject​: [PATCH] NULs in patterns
  Date​: Wed, 4 Feb 1998 01​:33​:51 -0500 (EST)
  Subject​: [PATCH] Configure on PerlIO
  Date​: Wed, 4 Feb 1998 01​:38​:43 -0500 (EST)
  Subject​: [PATCH] Avoid core dump on package alias
  Date​: Wed, 4 Feb 1998 15​:38​:42 -0500 (EST)
  Subject​: [PATCH] Fix name of $Foo​::{'Bar​::'}
  Date​: Wed, 4 Feb 1998 16​:37​:51 -0500 (EST)
 
  p4raw-id​: //depot/perl@​462

The change in question is this one​:

  As someone (?) recently observed, Perl's magical handling of filenames
  like "ls|" and "|tee foo bar" doesn't make a lot of sense in inplace
  editing mode (the -i command line option).
 
  This patch disables magic names in inplace mode. I tested it on a
  file named "echo abc|"; I ended up with files named "echo abc|" and
  "echo abc|~".

http​://www.xray.mpe.mpg.de/mailing-lists/perl5-porters/1998-02/msg00419.html

The change makes sense. Pretty much every "magical" filename under 2-arg open
is either for input or for output, hence (specifically for in-place editing)
it's unambiguously not useful to treat filenames as per 2-arg open rules.
The only possible exception is -, which is meaningful in those contexts as
STDIN and STDOUT, but

1) I don't think that it's worth having super-global-extra-magical exception
  rule for -
2) I checked GNU sed in inplace mode, and it stops treating - as STDIN

However, a bug remains. With inplace editing perl is still treating '-' as
meaning "write to STDOUT". ie​:

$ echo 'This is a file' >-
$ /Sandpit/5005_03g/bin/perl5.00503 -i -pwe0 -- -
This is a file
$ ls ./-~
ls​: ./-~​: No such file or directory

Contrast with​:

$ /Sandpit/5005_03g/bin/perl5.00503 -i -pwe0 -- ./-
$ ls ./-~
./-~

The fix itself is simple​:

Inline Patch
diff --git a/doio.c b/doio.c
index e2bfda5..65caa67 100644
--- a/doio.c
+++ b/doio.c
@@ -845,11 +845,6 @@ Perl_nextargv(pTHX_ GV *gv)
                 Gid_t filegid;
 
 		TAINT_PROPER("inplace open");
-		if (oldlen == 1 && *PL_oldname == '-') {
-		    setdefout(gv_fetchpvs("STDOUT", GV_ADD|GV_NOTQUAL,
-					  SVt_PVIO));
-		    return IoIFP(GvIOp(gv));
-		}
 #ifndef FLEXFILENAMES
 		filedev = PL_statbuf.st_dev;
 		fileino = PL_statbuf.st_ino;


1) Would anyone like to add tests for this? 2\) I don't think that it's documented anywhere that inplace editing makes   \<> behave differently\.   Would anyone like to patch the documentation?   \(Or show me where I'm wrong\)

Nicholas Clark

Perl Info


Site configuration information for perl 5.00503:

Configured by nick at Tue Jun 14 12:35:58 BST 2011.

Summary of my perl5 (5.0 patchlevel 5 subversion 3) configuration:
  Platform:
    osname=darwin, osvers=10.7.0, archname=darwin
    uname='darwin mouse-mill.local 10.7.0 darwin kernel version 10.7.0: sat jan 29 15:17:16 pst 2011; root:xnu-1504.9.37~1release_i386 i386 '
    hint=recommended, useposix=true, d_sigaction=define
    usethreads=undef useperlio=undef d_sfio=undef
  Compiler:
    cc='ccache gcc', optimize='-g', gccversion=4.2.1 (Apple Inc. build 5666) (dot 3)
    cppflags='-DDEBUGGING -I/opt/local/include'
    ccflags ='-DDEBUGGING -I/opt/local/include'
    stdchar='char', d_stdstdio=undef, usevfork=true
    intsize=4, longsize=8, ptrsize=8, doublesize=8
    d_longlong=define, longlongsize=8, d_longdbl=define, longdblsize=16
    alignbytes=8, usemymalloc=n, prototype=define
  Linker and Libraries:
    ld='env MACOSX_DEPLOYMENT_TARGET=10.3 cc', ldflags =' -L/usr/local/lib -L/opt/local/lib'
    libpth=/usr/local/lib /opt/local/lib /usr/lib
    libs=-lgdbm -ldbm -ldl -lm -lc
    libc=/usr/lib/libc.dylib, so=dylib, useshrplib=false, libperl=libperl.a
  Dynamic Linking:
    dlsrc=dl_dyld.xs, dlext=bundle, d_dlsymun=undef, ccdlflags=' '
    cccdlflags=' ', lddlflags=' -bundle -undefined dynamic_lookup -L/usr/local/lib -L/opt/local/lib'

Locally applied patches:
    


@INC for perl 5.00503:
    /Volumes/Stuff/Sandpit/5005_03g/lib/perl5/5.00503/darwin
    /Volumes/Stuff/Sandpit/5005_03g/lib/perl5/5.00503
    /Volumes/Stuff/Sandpit/5005_03g/lib/perl5/site_perl/5.005/darwin
    /Volumes/Stuff/Sandpit/5005_03g/lib/perl5/site_perl/5.005
    .


Environment for perl 5.00503:
    HOME=/Users/nick
    LANG (unset)
    LANGUAGE (unset)
    LD_LIBRARY_PATH (unset)
    LOGDIR (unset)
    PATH=/opt/local/bin:/opt/local/sbin:/Users/nick/bin:/opt/local/bin:/opt/local/sbin:/usr/bin:/bin:/usr/sbin:/sbin:/usr/local/bin:/usr/X11/bin:/opt/local/bin:/opt/local/sbin:/Users/nick/bin:/usr/local/sbin:/usr/local/sbin:/sbin:/usr/sbin
    PERL_BADLANG (unset)
    SHELL=/bin/bash

@toddr
Copy link
Member

@toddr toddr commented Feb 4, 2020

This is still mergeable, all we need are tests for this?

diff --git a/doio.c b/doio.c
index 9bbf2a4c57..ad1a5a75a2 100644
--- a/doio.c
+++ b/doio.c
@@ -1359,11 +1359,6 @@ Perl_nextargv(pTHX_ GV *gv, bool nomagicopen)
                 MAGIC *mg;
 
                TAINT_PROPER("inplace open");
-               if (oldlen == 1 && *PL_oldname == '-') {
-                   setdefout(gv_fetchpvs("STDOUT", GV_ADD|GV_NOTQUAL,
-                                         SVt_PVIO));
-                   return IoIFP(GvIOp(gv));
-               }
 #ifndef FLEXFILENAMES
                filedev = statbuf.st_dev;
                fileino = statbuf.st_ino;
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment