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 <[email protected]> >> 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 <[email protected]> > 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.
