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

[Pádraig Brady]

On 05/09/2012 11:14 AM, Jim Meyering wrote:
> 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 <address@hidden>
>> 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.
>> ---
> ...
>> diff --git a/src/stat.c b/src/stat.c
>> index b2e1030..d001cda 100644
>> --- a/src/stat.c
>> +++ b/src/stat.c
>> @@ -954,7 +954,7 @@ print_stat (char *pformat, size_t prefix_len, unsigned 
>> int m,
>>        out_uint_x (pformat, prefix_len, minor (statbuf->st_rdev));
>>        break;
>>      case 's':
>> -      out_uint (pformat, prefix_len, statbuf->st_size);
>> +      out_int (pformat, prefix_len, statbuf->st_size);
>>        break;
>>      case 'B':
>>        out_uint (pformat, prefix_len, ST_NBLOCKSIZE);
> 
> Thanks for all of that.
> I think Pádraig's question about dd's skip seeking to EOF on an
> actual tape device is the most important to address.
> 
> Your stat.c change is actually a bug fix, so I'd prefer to
> put it in a separate commit.  I did that for you.  Let me know
> if the change below is ok and I'll push it -- or you're welcome
> to make any change you'd like and push it yourself.
> 
>>From 0b816e06d0b3d0cc9b7d2f92b095145bfe7c5476 Mon Sep 17 00:00:00 2001
> From: Paul Eggert <address@hidden>
> Date: Wed, 9 May 2012 12:07:57 +0200
> Subject: [PATCH] stat: don't report negative file size as huge positive
>  number
> 
> * src/stat.c (print_stat): Use out_int, not out_uint for stat.st_size.
> * NEWS (Bug fixes): Mention it.
> ---
>  NEWS       | 2 ++
>  src/stat.c | 2 +-
>  2 files changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/NEWS b/NEWS
> index eb95404..2935276 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -22,6 +22,8 @@ GNU coreutils NEWS                                    -*- 
> outline -*-
>    split --number=C /dev/null no longer appears to infloop on GNU/Hurd
>    [bug introduced in coreutils-8.8]
> 
> +  stat no longer reports a negative file size as a huge positive number
> +
>  ** New features
> 
>    fmt now accepts the --goal=WIDTH (-g) option.
> diff --git a/src/stat.c b/src/stat.c
> index b2e1030..d001cda 100644
> --- a/src/stat.c
> +++ b/src/stat.c
> @@ -954,7 +954,7 @@ print_stat (char *pformat, size_t prefix_len, unsigned 
> int m,
>        out_uint_x (pformat, prefix_len, minor (statbuf->st_rdev));
>        break;
>      case 's':
> -      out_uint (pformat, prefix_len, statbuf->st_size);
> +      out_int (pformat, prefix_len, statbuf->st_size);
>        break;
>      case 'B':
>        out_uint (pformat, prefix_len, ST_NBLOCKSIZE);
> --
> 1.7.10.1.487.ga3935e6

For the record, stat(1) has output st_size as unsigned since the
initial implementation in fileutils-4.1.10.

I noticed that st_size is unsigned for 32 bit linux kernels
according to /usr/include/asm/stat.h, however my modern 32 kernel
gives EOVERFLOW for files in the 2-4G range, and thus shouldn't
put negative numbers in those fields. That used not be the case I think:
http://lkml.indiana.edu/hypermail/linux/kernel/0004.1/0343.html
Also other 32 bit environments might overflow here.

So I think this change is a net improvement.

cheers,
Pádraig.