[prev in list] [next in list] [prev in thread] [next in thread] 

List:       coreutils-bug
Subject:    bug#18449: "cat x >> x" error even when x is empty
From:       Paul Eggert <eggert () cs ! ucla ! edu>
Date:       2014-09-11 15:52:17
Message-ID: 5411C531.60305 () cs ! ucla ! edu
[Download RAW message or body]

Eric Blake wrote:
> Testing for an empty file is enough of an additional special case over
> the existing check for same files that I don't think it is worth it.

It's a really cheap check, as a system call is needed only when the 
input and output files are the same, and even then it's only an lseek 
with SEEK_CUR so it's all in-memory.  Plus, the POSIX 'cat' spec gives 
an example of using 'cat' to copy an empty regular file to itself, and 
(not unreasonably) says that should work.

As far as POSIX conformance goes, I don't think we need to worry about 
POSIXLY_CORRECT here.  We'll get POSIX fixed instead.  Eric has already 
started the ball rolling on that (thanks).  Though the POSIX fix should 
involve updating its example -- and quite possibly POSIX should continue 
to allow catting an empty file to itself.

I installed the attached patch and am marking this as done.  The 'cat' 
source code is simpler now, so that's a win anyway....

["0001-cat-allow-copying-empty-files-to-themselves.patch" (text/plain)]

From fc62159e7fb14f085f67f02b3f98af9e1e623a37 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Thu, 11 Sep 2014 08:45:32 -0700
Subject: [PATCH] cat: allow copying empty files to themselves

Problem reported by Vincent Lefevre in: http://bugs.gnu.org/18449
* src/cat.c (main): Allow copying an empty file to itself.
* tests/misc/cat-self.sh: New test.
* tests/local.mk (all_tests): Add it.
---
 src/cat.c              | 37 ++++++++++---------------------------
 tests/local.mk         |  1 +
 tests/misc/cat-self.sh | 33 +++++++++++++++++++++++++++++++++
 3 files changed, 44 insertions(+), 27 deletions(-)
 create mode 100755 tests/misc/cat-self.sh

diff --git a/src/cat.c b/src/cat.c
index 026348c..267864f 100644
--- a/src/cat.c
+++ b/src/cat.c
@@ -527,8 +527,8 @@ main (int argc, char **argv)
   /* I-node number of the output.  */
   ino_t out_ino;
 
-  /* True if the output file should not be the same as any input file.  */
-  bool check_redirection = true;
+  /* True if the output is a regular file.  */
+  bool out_isreg;
 
   /* Nonzero if we have ever read standard input.  */
   bool have_read_stdin = false;
@@ -637,25 +637,9 @@ main (int argc, char **argv)
     error (EXIT_FAILURE, errno, _("standard output"));
 
   outsize = io_blksize (stat_buf);
-  /* Input file can be output file for non-regular files.
-     fstat on pipes returns S_IFSOCK on some systems, S_IFIFO
-     on others, so the checking should not be done for those types,
-     and to allow things like cat < /dev/tty > /dev/tty, checking
-     is not done for device files either.  */
-
-  if (S_ISREG (stat_buf.st_mode))
-    {
-      out_dev = stat_buf.st_dev;
-      out_ino = stat_buf.st_ino;
-    }
-  else
-    {
-      check_redirection = false;
-#ifdef lint  /* Suppress 'used before initialized' warning.  */
-      out_dev = 0;
-      out_ino = 0;
-#endif
-    }
+  out_dev = stat_buf.st_dev;
+  out_ino = stat_buf.st_ino;
+  out_isreg = S_ISREG (stat_buf.st_mode) != 0;
 
   if (! (number || show_ends || squeeze_blank))
     {
@@ -704,14 +688,13 @@ main (int argc, char **argv)
 
       fdadvise (input_desc, 0, 0, FADVISE_SEQUENTIAL);
 
-      /* Compare the device and i-node numbers of this input file with
-         the corresponding values of the (output file associated with)
-         stdout, and skip this input file if they coincide.  Input
-         files cannot be redirected to themselves.  */
+      /* Don't copy a nonempty regular file to itself, as that would
+         merely exhaust the output device.  It's better to catch this
+         error earlier rather than later.  */
 
-      if (check_redirection
+      if (out_isreg
           && stat_buf.st_dev == out_dev && stat_buf.st_ino == out_ino
-          && (input_desc != STDIN_FILENO))
+          && lseek (input_desc, 0, SEEK_CUR) < stat_buf.st_size)
         {
           error (0, 0, _("%s: input file is output file"), infile);
           ok = false;
diff --git a/tests/local.mk b/tests/local.mk
index e0f1f84..1edaaf4 100644
--- a/tests/local.mk
+++ b/tests/local.mk
@@ -258,6 +258,7 @@ all_tests =					\
   tests/misc/wc-parallel.sh			\
   tests/misc/cat-proc.sh			\
   tests/misc/cat-buf.sh				\
+  tests/misc/cat-self.sh			\
   tests/misc/base64.pl				\
   tests/misc/basename.pl			\
   tests/misc/close-stdout.sh			\
diff --git a/tests/misc/cat-self.sh b/tests/misc/cat-self.sh
new file mode 100755
index 0000000..d8036d2
--- /dev/null
+++ b/tests/misc/cat-self.sh
@@ -0,0 +1,33 @@
+#!/bin/sh
+# Check that cat operates correctly when the input is the same as the output.
+
+# Copyright 2014 Free Software Foundation, Inc.
+
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+. "${srcdir=.}/tests/init.sh"; path_prepend_ ./src
+print_ver_ cat
+
+echo x >out || framework_failure_
+echo x >out1 || framework_failure_
+cat out >>out && fail=1
+compare out out1 || fail=1
+
+# This example is taken from the POSIX spec for 'cat'.
+echo x >doc || framework_failure_
+echo y >doc.end || framework_failure_
+cat doc doc.end >doc || fail=1
+compare doc doc.end || fail=1
+
+Exit $fail
-- 
1.9.3



[prev in list] [next in list] [prev in thread] [next in thread]