Re: sed -i should require write access (was: Bug in sed)

[Paul Eggert]

On 12/28/2010 08:55 AM, Bob Proulx wrote:
> That is the way Unix filesystem permissions work.

Yes, but still, it's bogus that "sed -i" overwrites a
read-only file.  That's not what users expect, and it's
not what the documentation says.  The documentation
says that -i "overwrites the file in place", which implies
that you must have write access to the file.

How about the following patch, to fix this?


>From 103818da056f5202ee1b89106fbe1b82f47860cd Mon Sep 17 00:00:00 2001
From: Paul Eggert <address@hidden(none)>
Date: Tue, 28 Dec 2010 09:48:55 -0800
Subject: [PATCH] sed -i no longer overwrites read-only files
 * NEWS, doc/sed-in.texi: Document this.
 * sed/execute.c (open_next_file): When editing in place, check that
 we have write access to the file.  Alter diagnostic slightly, since
 it could be that we can read the file but not write it.
 * sed/sed.c (input_file_mode): New var.
 (main): Set it.
 * sed/sed.h (input_file_mode): Declare it.

---
 ChangeLog       |   11 +++++++++++
 NEWS            |    2 ++
 doc/sed-in.texi |    3 +++
 sed/execute.c   |    4 ++--
 sed/sed.c       |    6 ++++++
 sed/sed.h       |    4 ++++
 6 files changed, 28 insertions(+), 2 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 53cb176..361a2b8 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,14 @@
+2010-12-28  Paul Eggert  <address@hidden>
+
+       sed -i no longer overwrites read-only files
+       * NEWS, doc/sed-in.texi: Document this.
+       * sed/execute.c (open_next_file): When editing in place, check that
+       we have write access to the file.  Alter diagnostic slightly, since
+       it could be that we can read the file but not write it.
+       * sed/sed.c (input_file_mode): New var.
+       (main): Set it.
+       * sed/sed.h (input_file_mode): Declare it.
+       
 2010-11-13  Jari Aalto  <address@hidden>
 
        * doc/sed.x: Order commands alphabetically.
diff --git a/NEWS b/NEWS
index 89ea757..2826700 100644
--- a/NEWS
+++ b/NEWS
@@ -1,5 +1,7 @@
 Sed 4.2.2
 
+* -i no longer overwrites read-only files
+
 * fix endless loop on incomplete multibyte sequences
 
 * -u also does unbuffered input, rather than unbuffered output only
diff --git a/doc/sed-in.texi b/doc/sed-in.texi
index b99695d..59ead50 100644
--- a/doc/sed-in.texi
+++ b/doc/sed-in.texi
@@ -270,6 +270,9 @@ directory (provided the directory already exists).
 If no extension is supplied, the original file is
 overwritten without making a backup.
 
+When using this option, you must have write access to
+the original file.
+
 @item -l @var{N}
 @itemx address@hidden
 @opindex -l
diff --git a/sed/execute.c b/sed/execute.c
index 14fd4e4..ed5afc5 100644
--- a/sed/execute.c
+++ b/sed/execute.c
@@ -703,10 +703,10 @@ open_next_file(name, input)
       clearerr(stdin); /* clear any stale EOF indication */
       input->fp = ck_fdopen (fileno (stdin), "stdin", read_mode, false);
     }
-  else if ( ! (input->fp = ck_fopen(name, read_mode, false)) )
+  else if ( ! (input->fp = ck_fopen(name, input_file_mode, false)) )
     {
       const char *ptr = strerror(errno);
-      fprintf(stderr, _("%s: can't read %s: %s\n"), myname, name, ptr);
+      fprintf(stderr, _("%s: can't open %s: %s\n"), myname, name, ptr);
       input->read_fn = read_always_fail; /* a redundancy */
       ++input->bad_count;
       return;
diff --git a/sed/sed.c b/sed/sed.c
index 6de1280..7b74ba4 100644
--- a/sed/sed.c
+++ b/sed/sed.c
@@ -87,6 +87,10 @@ char *in_place_extension = NULL;
 char *read_mode = "rt";
 char *write_mode = "w";
 
+/* The mode to use when reading an input file, which may be edited in place;
+   one of "rt", "rb", "r+t", "r+b".  */
+char *input_file_mode = "rt";
+
 /* Do we need to be pedantically POSIX compliant? */
 enum posixicity_types posixicity;
 
@@ -281,6 +285,7 @@ main(argc, argv)
              strcpy (in_place_extension + 1, optarg);
            }
 
+         input_file_mode = strchr (input_file_mode, 'b') ? "r+b" : "r+t";
          break;
 
        case 'l':
@@ -294,6 +299,7 @@ main(argc, argv)
         case 'b':
          read_mode = "rb";
          write_mode = "wb";
+         input_file_mode = in_place_extension ? "r+b" : "rb";
          break;
 
        /* Undocumented, for compatibility with BSD sed.  */
diff --git a/sed/sed.h b/sed/sed.h
index f8ccccc..c7c6f3f 100644
--- a/sed/sed.h
+++ b/sed/sed.h
@@ -237,6 +237,10 @@ extern char *in_place_extension;
 extern char *read_mode;
 extern char *write_mode;
 
+/* The mode to use when reading an input file, which may be edited in place;
+   one of "rt", "rb", "r+t", "r+b".  */
+extern char *input_file_mode;
+
 /* Should we use EREs? */
 extern bool use_extended_syntax_p;
 
-- 
1.7.1