Subject: bug#6268: Suggestion: truncate should allow -r and -s options together

From: Pádraig Brady <[email-masked]>
Date: Sat, 29 May 2010 01:03:08 +0100

On 28/05/10 14:17, Pádraig Brady wrote:
>> what about --ref=non-regular-file ?
>>
>> Perhaps truncate should refuse to use st_size info from
>> a non-regular file.
>>
>> Here's a pathological example:
>>
>>   $ echo abcdefgh > bar
>>   $ strace -e ftruncate ./truncate --ref=/dev/tty bar
>>   ftruncate(3, 0)                         = 0
>>   $ wc -c bar
>>   0 bar
>>
>> It's obvious that such an example is not likely in practice, but since
>> currently truncate's --ref uses stat, it would follow a symlink, too.
>>
>> For a directory you'd get unportable and probably surprising results.
>> Using such a reference size deserves at least a warning.
> 
> Yes that's safer. st_size is only defined for regular files
> (or shared mem), so I'll only allow regular files.
> I'll push a separate patch soon.
> 
> Since we're only referencing the size, there is the argument
> that we should be trying harder to get the size of block devices
> for example.  There are portability issues with that though,
> and doing outside of truncate is easy enough, so I'll leave that for now.

I actually removed all ignoring of errors for
non regular files, as well as not allowing
one to reference st_size for non regular files.

I'll push the attached tomorrow sometime.

cheers,
Pádraig.
>From 0b9588c10cb3783ade69892302eb0d1379866af6 Mon Sep 17 00:00:00 2001
From: =?utf-8?q?P=C3=A1draig=20Brady?= <[email-masked]>
Date: Fri, 28 May 2010 19:25:23 +0100
Subject: [PATCH] truncate: improve handling of non regular files

Previously we copied `dd` and suppressed error messages
when truncating neither regular files or shared mem objects.
This was valid for `dd`, as truncation is ancillary to copying
it may also do, but for `truncate` we should display all errors.
Also we used the st_size from non regular files which is undefined,
so we display an error when the user tries this.

* src/truncate (do_truncate):  Error when referencing the size
of non regular files or non shared memory objects.  Display all
errors returned by ftruncate().
(main): Error when referencing the size of non regular files or
non shared memory objects.  Don't suppress error messages for
any file types that can't be opened for writing.
* tests/misc/truncate-dir-fail: Check that referencing the
size of a directory is not supported.
* tests/misc/truncate-fifo: Ensure the test doesn't hang
by using the `timeout` command.  Don't test the return from
running ftruncate on the fifo as it's system dependent as
to whether this fails or not.
---
 src/truncate.c               |   59 +++++++++++++-----------------------------
 tests/misc/truncate-dir-fail |    3 ++
 tests/misc/truncate-fifo     |    6 ++--
 3 files changed, 24 insertions(+), 44 deletions(-)

diff --git a/src/truncate.c b/src/truncate.c
index 08090ab..cfb5747 100644
--- a/src/truncate.c
+++ b/src/truncate.c
@@ -160,21 +160,21 @@ do_ftruncate (int fd, char const *fname, off_t ssize, off_t rsize,
     {
       uintmax_t const fsize = rsize < 0 ? sb.st_size : rsize;
 
-      if (rsize < 0 && sb.st_size < 0)
+      if (rsize < 0) /* fstat used above to get size.  */
         {
-          /* Complain only for a regular file, a directory,
-             or a shared memory object, as POSIX 1003.1-2004 specifies
-             ftruncate's behavior only for these file types.  */
-          if (S_ISREG (sb.st_mode) || S_ISDIR (sb.st_mode)
-              || S_TYPEISSHM (&sb))
+          if (!S_ISREG (sb.st_mode) && !S_TYPEISSHM (&sb))
             {
-              /* overflow is the only reason I can think
-                 this would ever go negative for the above types */
+              error (0, 0, _("cannot get the size of %s"), quote (fname));
+              return 1;
+            }
+          if (sb.st_size < 0)
+            {
+              /* Sanity check. Overflow is the only reason I can think
+                 this would ever go negative. */
               error (0, 0, _("%s has unusable, apparently negative size"),
                      quote (fname));
               return 1;
             }
-          return 0;
         }
 
       if (rel_mode == rm_min)
@@ -215,26 +215,10 @@ do_ftruncate (int fd, char const *fname, off_t ssize, off_t rsize,
 
   if (ftruncate (fd, nsize) == -1)      /* note updates mtime & ctime */
     {
-      /* Complain only when ftruncate fails on a regular file, a
-         directory, or a shared memory object, as POSIX 1003.1-2004
-         specifies ftruncate's behavior only for these file types.
-         For example, do not complain when Linux kernel 2.4 ftruncate
-         fails on /dev/fd0.  */
-      int const ftruncate_errno = errno;
-      if (fstat (fd, &sb) != 0)
-        {
-          error (0, errno, _("cannot fstat %s"), quote (fname));
-          return 1;
-        }
-      else if (S_ISREG (sb.st_mode) || S_ISDIR (sb.st_mode)
-               || S_TYPEISSHM (&sb))
-        {
-          error (0, ftruncate_errno,
-                 _("truncating %s at %" PRIdMAX " bytes"), quote (fname),
-                 (intmax_t) nsize);
-          return 1;
-        }
-      return 0;
+      error (0, errno,
+             _("truncating %s at %" PRIdMAX " bytes"), quote (fname),
+             (intmax_t) nsize);
+      return 1;
     }
 
   return 0;
@@ -362,9 +346,13 @@ main (int argc, char **argv)
 
   if (ref_file)
     {
+      /* FIXME: Maybe support getting size of block devices.  */
       struct stat sb;
       if (stat (ref_file, &sb) != 0)
         error (EXIT_FAILURE, errno, _("cannot stat %s"), quote (ref_file));
+      if (!S_ISREG (sb.st_mode) && !S_TYPEISSHM (&sb))
+        error (EXIT_FAILURE, 0, _("cannot get the size of %s"),
+               quote (ref_file));
       if (!got_size)
         size = sb.st_size;
       else
@@ -384,18 +372,7 @@ main (int argc, char **argv)
              `truncate -s0 .` should gen EISDIR error */
           if (!(no_create && errno == ENOENT))
             {
-              int const open_errno = errno;
-              struct stat sb;
-              if (stat (fname, &sb) == 0)
-                {
-                  /* Complain only for a regular file, a directory,
-                     or a shared memory object, as POSIX 1003.1-2004 specifies
-                     ftruncate's behavior only for these file types.  */
-                  if (!S_ISREG (sb.st_mode) && !S_ISDIR (sb.st_mode)
-                      && !S_TYPEISSHM (&sb))
-                    continue;
-                }
-              error (0, open_errno, _("cannot open %s for writing"),
+              error (0, errno, _("cannot open %s for writing"),
                      quote (fname));
               errors++;
             }
diff --git a/tests/misc/truncate-dir-fail b/tests/misc/truncate-dir-fail
index d2ecf2f..fe33857 100755
--- a/tests/misc/truncate-dir-fail
+++ b/tests/misc/truncate-dir-fail
@@ -26,4 +26,7 @@ fi
 # truncate on dir not allowed
 truncate -s+0 . && fail=1
 
+# getting the size of a dir is not allowed
+truncate -r. file && fail=1
+
 Exit $fail
diff --git a/tests/misc/truncate-fifo b/tests/misc/truncate-fifo
index 8e2276d..12d674c 100755
--- a/tests/misc/truncate-fifo
+++ b/tests/misc/truncate-fifo
@@ -1,5 +1,5 @@
 #!/bin/sh
-# Make sure truncate works on fifos without hanging or errors
+# Make sure truncate works on fifos without hanging
 
 # Copyright (C) 2008-2010 Free Software Foundation, Inc.
 
@@ -25,7 +25,7 @@ fi
 
 mkfifo_or_skip_ "fifo"
 
-
-truncate -s0 "fifo" || fail=1
+timeout 10 truncate -s0 "fifo"
+test "$?" = 124 && fail=1
 
 Exit $fail
-- 
1.6.2.5