Subject: Re: bug#11424: coreutils: split tests hang on /dev/zero on GNU/Hurd

From: Pádraig Brady <[email-masked]>
Date: Tue, 08 May 2012 23:57:25 +0100

On 05/08/2012 05:27 PM, Paul Eggert wrote:
> On 05/08/2012 01:39 AM, Jim Meyering wrote:
>> I went ahead and pushed the less-invasive fix.
> 
> Hmm, I don't see this on Savannah; is this part
> of the problem where usable_st_size got pushed?
> 
>> Your behavior-changing one is more than welcome, too.
> 
> I came up with a better idea, and propose this patch
> instead.  The idea is to fall back on lseek if
> st_size is not reliable.  This allows the programs
> to work in more cases, including the case in question.
> One test case needs to be removed because it assumes
> a command must fail, where it now typically works.
> 
> 
>>>From bba6f199f621fb434c5df09a0b0278a081be87e2 Mon Sep 17 00:00:00 2001
> From: Paul Eggert <[email-masked]>
> Date: Tue, 8 May 2012 09:22:22 -0700
> Subject: [PATCH] maint: handle file sizes more reliably
> 
> Problem reported by Samuel Thibault in <http://debbugs.gnu.org/11424>.
> * NEWS: Document this.
> * src/dd.c (skip):
> * src/split.c (main):
> * src/truncate.c (do_ftruncate, main):
> On files where st_size is not portable, fall back on using lseek
> with SEEK_END to determine the size.  Although strictly speaking
> POSIX says the behavior is implementation-defined, in practice
> if lseek returns a nonnegative value it's a reasonable one to
> use for the file size.
> * src/dd.c (dd_copy): It's OK to truncate shared memory objects.
> * src/du.c (duinfo_add): Check for overflow.
> (print_only_size): Report overflow.
> (process_file): Ignore negative file sizes in the --apparent-size case.
> * src/od.c (skip): Fix comment about st_size.
> * src/stat.c (print_stat): Don't report negative sizes as huge
> positive ones.
> * src/system.h (usable_st_size): Symlinks have reliable st_size too.
> * tests/misc/truncate-dir-fail: Don't assume that getting the size
> of a dir is not allowed, as it's now allowed on many platforms,
> e.g., GNU/Linux.
> ---
>  NEWS                         |    3 ++
>  src/dd.c                     |   17 +++++++++---
>  src/du.c                     |   12 ++++++---
>  src/od.c                     |    3 +-
>  src/split.c                  |    7 +++++
>  src/stat.c                   |    2 +-
>  src/system.h                 |    3 +-
>  src/truncate.c               |   56 ++++++++++++++++++++++++++++++-----------
>  tests/misc/truncate-dir-fail |    3 --
>  9 files changed, 76 insertions(+), 30 deletions(-)
> 
> diff --git a/NEWS b/NEWS
> index 2dc6531..c911d52 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -21,6 +21,9 @@ GNU coreutils NEWS                                    -*- outline -*-
>  
>  ** New features
>  
> +  dd, split, and truncate now allow any seekable files in situations
> +  where the file size is needed, instead of insisting on regular files.
> +
>    fmt now accepts the --goal=WIDTH (-g) option.
>  
>  ** Changes in behavior
> diff --git a/src/dd.c b/src/dd.c
> index 4626de2..75109bc 100644
> --- a/src/dd.c
> +++ b/src/dd.c
> @@ -1542,9 +1542,18 @@ skip (int fdesc, char const *file, uintmax_t records, size_t blocksize,
>        if (fdesc == STDIN_FILENO)
>          {
>             struct stat st;
> +           off_t file_size;
>             if (fstat (STDIN_FILENO, &st) != 0)
>               error (EXIT_FAILURE, errno, _("cannot fstat %s"), quote (file));
> -           if (S_ISREG (st.st_mode) && st.st_size < (input_offset + offset))
> +           if (usable_st_size (&st))
> +             file_size = st.st_size;
> +           else
> +             {
> +               file_size = skip_via_lseek (file, fdesc, 0, SEEK_END);
> +               if (skip_via_lseek (file, fdesc, offset, SEEK_CUR) < 0)
> +                 error (EXIT_FAILURE, errno, _("%s: cannot skip"), quote (file));
> +             }
> +           if (0 <= file_size && file_size < input_offset + offset)
>               {
>                 /* When skipping past EOF, return the number of _full_ blocks
>                  * that are not skipped, and set offset to EOF, so the caller

Might SEEK_END have implications for tape devices,
where we might go to the end even if only operating on the start of the device?

This thread reminded me of an old one where I previously wondered if
one could refactor out a stat_size() that would return unsigned and
thus simplify many callers?
http://lists.gnu.org/archive/html/bug-coreutils/2009-01/msg00069.html

I think it was suggested in that thread that POSIX will
return a positive st_size for those st_modes?

Something like:

uintmax_t stat_size (struct stat const *statp, int fd)
{
  uintmax_t size = UINTMAX_MAX; /* Error.  */

  struct stat sb;
  if (! statp)
    {
      if (fd > 0)
        {
          if (fstat (fd, &sb) != 0)
              return size;
          statp = &sb;
        }
      else
        return size;
    }

  if (S_ISREG (statp->st_mode) || S_ISLNK (statp->st_mode)
      || S_TYPEISSHM (&stat_buf) || S_TYPEISTMO (&stat_buf))
    size = statp->st_size;
  else if (fd > 0)
    ; /* Maybe do something with SEEK_END or BLKGETSIZE64 ? */
}

cheers,
Pádraig.