bug#11424: coreutils: split tests hang on /dev/zero on GNU/Hurd
Samuel Thibault wrote:
Since some time, coreutils fails in split tests on GNU/Hurd. More
precisely, these two:
split/filter:55
split --filter=head -c1 /dev/null -n 1 /dev/zero
split/l-chunk:39
split -n l/2 /dev/zero
It seems that these two tests assume that split will stop by itself when
given /dev/zero as input. It does so on Linux, because /dev/zero there
has stat_buf.st_size equal to 0, and thus split does just one loop, but
on GNU/Hurd /dev/zero has stat_buf.st_size equal to LONG_MAX, and thus
split just loops for a very long time. Posix doesn't seem to talk much
about what should be done here, but it seems to me that coreutils tests
should not assume size being zero, and for instance use dd to fetch only
the required bytes from /dev/zero.
Hi Samuel,
The real problem is that split was using stat.st_size from non-regular
files: that is not defined.
Here is a patch:
From 0a63df4b669faf0585beab09f4b177c39d557b21 Mon Sep 17 00:00:00 2001
From: Jim Meyering meyer...@redhat.com
Date: Mon, 7 May 2012 09:32:00 +0200
Subject: [PATCH] split: avoid apparent infloop when splitting /dev/zero w/-n
on the Hurd
* src/split.c (main): Use stat.st_size only for regular files.
Samuel Thibault reported in http://bugs.gnu.org/11424 that the
/dev/zero-splitting tests would appear to infloop on GNU/Hurd,
because /dev/zero's st_size is LONG_MAX. It was only a problem
when using the --number (-n) option.
* NEWS (Bug fixes): Mention it.
This bug was introduced with the --number option, via
commit v8.7-25-gbe10739
---
NEWS| 3 +++
src/split.c | 4 +++-
2 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/NEWS b/NEWS
index fd563c0..7563646 100644
--- a/NEWS
+++ b/NEWS
@@ -19,6 +19,9 @@ GNU coreutils NEWS-*-
outline -*-
was particularly easy to trigger, since there, the removal of D could
precede the initial stat. [This bug was present in the beginning.]
+ split --number=C /dev/null no longer appears to infloop on GNU/Hurd
+ [bug introduced in coreutils-8.8]
+
** New features
fmt now accepts the --goal=WIDTH (-g) option.
diff --git a/src/split.c b/src/split.c
index 99f6390..062aede 100644
--- a/src/split.c
+++ b/src/split.c
@@ -1339,7 +1339,9 @@ main (int argc, char **argv)
error (EXIT_FAILURE, errno, %s, infile);
if (in_blk_size == 0)
in_blk_size = io_blksize (stat_buf);
- file_size = stat_buf.st_size;
+
+ /* stat.st_size is valid only for regular files. For others, use 0. */
+ file_size = S_ISREG (stat_buf.st_mode) ? stat_buf.st_size : 0;
if (split_type == type_chunk_bytes || split_type == type_chunk_lines)
{
--
1.7.10.1.457.g8275905
bug#11424: coreutils: split tests hang on /dev/zero on GNU/Hurd
Thanks, here's a quick cut at a patch to 'split' to fix the problem.
Also, the test cases also need to be adjusted, so as not to attempt to
split -n on a device.
diff --git a/src/split.c b/src/split.c
index 99f6390..8b966bc 100644
--- a/src/split.c
+++ b/src/split.c
@@ -1344,7 +1344,9 @@ main (int argc, char **argv)
if (split_type == type_chunk_bytes || split_type == type_chunk_lines)
{
off_t input_offset = lseek (STDIN_FILENO, 0, SEEK_CUR);
- if (input_offset 0)
+ if (input_offset 0
+ || ! (S_ISREG (stat_buf.st_mode)
+|| S_TYPEISSHM (stat_buf) || S_TYPEISTMO (stat_buf)))
error (EXIT_FAILURE, 0, _(%s: cannot determine file size),
quote (infile));
file_size -= input_offset;
I audited coreutils for other misuses of st_size and found some;
here are additional quick cuts at patches that look like they're needed.
I didn't attempt to look for missed optimizations; just errors.
diff --git a/src/dd.c b/src/dd.c
index 4626de2..147b69d 100644
--- a/src/dd.c
+++ b/src/dd.c
@@ -1544,7 +1544,8 @@ skip (int fdesc, char const *file, uintmax_t records,
size_t blocksize,
struct stat st;
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 ((S_ISREG (st.st_mode) || S_TYPEISSHM (st) || S_TYPEISTMO (st))
+st.st_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
@@ -2104,8 +2105,8 @@ dd_copy (void)
}
}
- /* If the last write was converted to a seek, then for a regular file,
- ftruncate to extend the size. */
+ /* If the last write was converted to a seek, then for a regular file
+ or shared memory object, ftruncate to extend the size. */
if (final_op_was_seek)
{
struct stat stdout_stat;
@@ -2114,7 +2115,7 @@ dd_copy (void)
error (0, errno, _(cannot fstat %s), quote (output_file));
return EXIT_FAILURE;
}
- if (S_ISREG (stdout_stat.st_mode))
+ if (S_ISREG (stdout_stat.st_mode) || S_TYPEISSHM (stdout_stat))
{
off_t output_offset = lseek (STDOUT_FILENO, 0, SEEK_CUR);
if (output_offset stdout_stat.st_size)
diff --git a/src/du.c b/src/du.c
index 41c9535..7333941 100644
--- a/src/du.c
+++ b/src/du.c
@@ -99,7 +99,8 @@ duinfo_set (struct duinfo *a, uintmax_t size, struct timespec
tmax)
static inline void
duinfo_add (struct duinfo *a, struct duinfo const *b)
{
- a-size += b-size;
+ uintmax_t sum = a-size + b-size;
+ a-size = a-size = sum ? sum : UINTMAX_MAX;
if (timespec_cmp (a-tmax, b-tmax) 0)
a-tmax = b-tmax;
}
@@ -370,8 +371,11 @@ static void
print_only_size (uintmax_t n_bytes)
{
char buf[LONGEST_HUMAN_READABLE + 1];
- fputs (human_readable (n_bytes, buf, human_output_opts,
- 1, output_block_size), stdout);
+ fputs ((n_bytes == UINTMAX_MAX
+ ? _(Infinity)
+ : human_readable (n_bytes, buf, human_output_opts,
+1, output_block_size)),
+ stdout);
}
/* Print size (and optionally time) indicated by *PDUI, followed by STRING. */
@@ -495,7 +499,7 @@ process_file (FTS *fts, FTSENT *ent)
duinfo_set (dui,
(apparent_size
- ? sb-st_size
+ ? MAX (0, sb-st_size)
: (uintmax_t) ST_NBLOCKS (*sb) * ST_NBLOCKSIZE),
(time_type == time_mtime ? get_stat_mtime (sb)
: time_type == time_atime ? get_stat_atime (sb)
diff --git a/src/od.c b/src/od.c
index 7593796..a25f965 100644
--- a/src/od.c
+++ b/src/od.c
@@ -983,8 +983,7 @@ skip (uintmax_t n_skip)
if (fstat (fileno (in_stream), file_stats) == 0)
{
- /* The st_size field is valid only for regular files
- (and for symbolic links, which cannot occur here).
+ /* The st_size field is valid for regular files.
If the number of bytes left to skip is larger than
the size of the current file, we can decrement n_skip
and go on to the next file. Skip this optimization also
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);
diff --git a/src/truncate.c b/src/truncate.c
index 9b847d2..eaef598 100644
--- a/src/truncate.c
+++ b/src/truncate.c
@@ -161,7 +161,8 @@ do_ftruncate (int fd,
bug#11424: coreutils: split tests hang on /dev/zero on GNU/Hurd
On 05/07/2012 12:46 AM, Jim Meyering wrote: + + /* stat.st_size is valid only for regular files. For others, use 0. */ + file_size = S_ISREG (stat_buf.st_mode) ? stat_buf.st_size : 0; Is it right to use 0 there, for non-regular files? Won't later code compute incorrect sizes in that case? Also, as a nit, stat.st_size is also valid for SHM and TMO files (this was in the patch I just sent).
bug#11424: coreutils: split tests hang on /dev/zero on GNU/Hurd
Paul Eggert wrote:
On 05/07/2012 12:46 AM, Jim Meyering wrote:
+
+ /* stat.st_size is valid only for regular files. For others, use 0. */
+ file_size = S_ISREG (stat_buf.st_mode) ? stat_buf.st_size : 0;
Is it right to use 0 there, for non-regular files?
Won't later code compute incorrect sizes in that case?
Hi Paul,
I agree that more change is required and do prefer the direction your
patches suggest. However, to fix the Hurd/infloop with minimal
impact elsewhere, I have a slight preference for my small change.
I.e. continuing to operate on non-regular files with --number we
don't have to change the split --number tests that operate on /dev/zero.
Then, introducing the behavior change (with your follow-on patch) can
be independent of the bug fix commit.
I do admit that without being able to determine a size up front, there's
little point in using that option, so your patch (reject files with
unusable stat.st_size) is required.
With or without my patch on Linux/GNU, if you split /dev/zero,
it sets file_size = 0, so at least for the tested cases
I don't think that patch introduces a regression.
Also, as a nit, stat.st_size is also valid for
SHM and TMO files (this was in the patch I just sent).
Good point.
Do you feel like adding something like this to system.h
and completing your patch?
/* Return a boolean indicating whether sb-st_size is defined. */
static inline bool
usable_st_size (struct stat const *sb)
{
return S_ISREG (sb-st_mode) || S_TYPEISSHM (sb) || S_TYPEISTMO (sb);
}
bug#11100: Racy code in copy.c
* Jim Meyering (j...@meyering.net) [20120504 17:30]: If there's a bugzilla reference for this, let me know and I'll add it to the commit log. There is, but as it's a SLES bug it's only open for SUSE employees and customers and thus useless for a coreutils commit log. I'll instead reference the commit from said bug report. Philipp
bug#11100: Racy code in copy.c
Philipp Thomas wrote: * Jim Meyering (j...@meyering.net) [20120504 17:30]: If there's a bugzilla reference for this, let me know and I'll add it to the commit log. There is, but as it's a SLES bug it's only open for SUSE employees and customers and thus useless for a coreutils commit log. I'll instead reference the commit from said bug report. Ok. I've pushed that change.
bug#11427: cp 8.16 not writing through, writing over
Create dangling symlink: $ ln -s foo bar Attempt to write over it with cp: $ \cp -i /etc/issue bar cp: not writing through dangling symlink 'bar' In the past, it would ask me if I wanted to replace bar. (As desired.) The error message makes me think that it is thinking I am writing through a dangling symlink to a directory. But that gets a different error message, which seems fine: $ \cp -i /etc/issue bar/baz cp: cannot create regular file 'bar/baz': No such file or directory coreutils 8.16, compiled from original source on CentOS 5.8 (libc 2.5, it seems). karl
bug#11429: regarding conflict type error in compilation process
Hi, i am getting an error in compilation process in Linux environment,this error is like conflicting types for ablkcnt_ta and previous declaration of blkcnt_t was here in Linux envioronment.please give me the instructions to solve this problem. thanksregards mahesh
bug#11427: cp 8.16 not writing through, writing over
Karl Berry wrote: Create dangling symlink: $ ln -s foo bar Attempt to write over it with cp: $ \cp -i /etc/issue bar cp: not writing through dangling symlink 'bar' In the past, it would ask me if I wanted to replace bar. (As desired.) Hi Karl, When I try that in an empty directory and using coreutils-6.7's cp (which predates the change mentioned below), it does this: $ ln -s foo bar $ env cp -i /etc/issue bar: cp: cannot create regular file `bar': File exists Maybe you want to use --backup? The error message makes me think that it is thinking I am writing through a dangling symlink to a directory. But that gets a different error message, which seems fine: $ \cp -i /etc/issue bar/baz cp: cannot create regular file 'bar/baz': No such file or directory coreutils 8.16, compiled from original source on CentOS 5.8 (libc 2.5, it seems). [quick answer: set POSIXLY_CORRECT] That behavior change dates back to 2007. Here's the NEWS snippet: * Noteworthy changes in release 6.9.90 (2007-12-01) [beta] ... ** Changes in behavior cp, by default, refuses to copy through a dangling destination symlink Set POSIXLY_CORRECT if you require the old, risk-prone behavior. There's more detail here: http://git.sv.gnu.org/cgit/coreutils.git/commit/?id=2bdc48121916ab0d7bb7d and even more on the mailing list just before I made that change.
bug#11429: regarding conflict type error in compilation process
Could you give more details about your problem? Please start by saying what source code you downloaded, what system you're using (hardware and software), what commands you ran, and exactly what the failed output was.
