Subject: Re: bug#21249: "sed -i '...' -" in git head

From: Jim Meyering <[email-masked]>
Date: Sat, 2 Jan 2016 19:25:52 -0800

On Thu, Aug 13, 2015 at 7:15 AM, Stephane Chazelas
<[email-masked]> wrote:
> Hello,
>
> about this commit:
>
>> commit c033bdee411128dfebfea1974d1ee3c1d9eac572
>> Author: Jim Meyering <[email-masked]>
>> Date:   Sat Jun 20 07:38:49 2015 -0700
>>
>>     sed -i: do not treat "-" as a file name
>
> the behaviour was aligned with perl's (where that syntax derives
> from).
>
> In perl, perl -pi -e 's/../../' -- *
>
> or perl -pi -e 's/../../' -- "$file"
>
> is known to be /reliable/ (work regardless of the value of $file
> (while without -i it's not, see
> https://unix.stackexchange.com/questions/170013/security-implications-of-running-perl-ne
> (-, cmd|, <file... are a problem there))
>
> That was also /safe/ in sed before that change. Treating "-" as
> stdin with -i doesn't make sense as it doesn't make sense to
> edit stdin "in-place".
>
> Now that means it breaks scripts that do:
> sed -i '...' -- "$file"
> expecting it modify $file regardless of the name of $file. Now,
> one has to do:
>
> case $file in
>  -) file=./-
> esac
> sed -i '...' -- "$file"
>
> for no good reason.
>
> IMO, that change only has negative consequences.

Thank you for the report and good argument.
I plan to revert that change with the attached patch:
From 61c0a53ec997e128fe5c976eda7412571409410a Mon Sep 17 00:00:00 2001
From: Jim Meyering <[email-masked]>
Date: Sun, 27 Dec 2015 10:33:06 -0800
Subject: [PATCH] revert v4.2.2-87-gc033bde, to make sed -i treat "-" as a file
 name

Stephane Chazelas made a fine case for why sed's --in-place (-i)
should treat "-" as a file name, and not as standard input in
http://debbugs.gnu.org/21249

There was no specific test for the prior behavior, so rather than
letting the revert remove the sole test, adapt and retain it:
* testsuite/in-place-hyphen.sh: However, retain and adapt this
file to test for the restored behavior.
* testsuite/Makefile.am: Retain this change, too.
---
 NEWS                         |  3 ---
 sed/execute.c                |  5 +----
 testsuite/in-place-hyphen.sh | 11 ++++++-----
 3 files changed, 7 insertions(+), 12 deletions(-)

diff --git a/NEWS b/NEWS
index 7b5c6f9..45ae7ae 100644
--- a/NEWS
+++ b/NEWS
@@ -55,9 +55,6 @@ GNU sed NEWS                                    -*- outline -*-
   has been listed in the documentation as a failed experiment for at
   least 10 years.  That command is now removed.

-  sed -i s/a/b/ - now evokes an error, rather than attempting to operate
-  on the file named ./-
-
 ** Build-related

   "make dist" now builds .tar.xz files, rather than .tar.gz ones.
diff --git a/sed/execute.c b/sed/execute.c
index e5b66c2..3fe15cb 100644
--- a/sed/execute.c
+++ b/sed/execute.c
@@ -550,11 +550,8 @@ open_next_file(const char *name, struct input *input)
   buffer.length = 0;

   input->in_file_name = name;
-  if (name[0] == '-' && name[1] == '\0')
+  if (name[0] == '-' && name[1] == '\0' && !in_place_extension)
     {
-      if (in_place_extension)
-        panic(_("couldn't edit %s: is a terminal"), name);
-
       clearerr(stdin);	/* clear any stale EOF indication */
 #if defined WIN32 || defined _WIN32 || defined __CYGWIN__ \
   || defined MSDOS || defined __EMX__
diff --git a/testsuite/in-place-hyphen.sh b/testsuite/in-place-hyphen.sh
index ae1153b..f44efd2 100755
--- a/testsuite/in-place-hyphen.sh
+++ b/testsuite/in-place-hyphen.sh
@@ -1,5 +1,5 @@
 #!/bin/sh
-# Verify that "-" is no longer treated as a file name with --in-place.
+# Verify that "-" is treated as a file name with --in-place.

 # Copyright (C) 2015-2016 Free Software Foundation, Inc.

@@ -18,12 +18,13 @@
 . "${srcdir=.}/init.sh"; path_prepend_ ../sed
 print_ver_ sed

-echo "sed: couldn't edit -: is a terminal" > exp-err || framework_failure_
+echo abc > ./- || framework_failure_
+echo aXc > exp-out || framework_failure_

 fail=0
-sed -i 's/a/b/' - > out 2> err && fail=1
+sed -i 's/b/X/' - > out 2> err || fail=1

-compare /dev/null out || fail=1
-compare exp-err err || fail=1
+compare exp-out ./- || fail=1
+compare /dev/null err || fail=1

 Exit $fail
-- 
2.6.2