diff mbox

generic/397: be compatible with ignored SIGPIPE

Message ID 20170612211528.45666-1-ebiggers3@gmail.com (mailing list archive)
State New, archived
Headers show

Commit Message

Eric Biggers June 12, 2017, 9:15 p.m. UTC
From: Eric Biggers <ebiggers@google.com>

If generic/397 is executed in an environment with SIGPIPE ignored, it
fails because the 'yes' program prints an error message:

    yes: standard output: Broken pipe
    yes: write error

This can be reproduced with:

    trap '' SIGPIPE; ./check generic/397

Fix it by generating the string of 255 y's using just 'head' and 'tr'
instead of 'yes', 'head', and 'tr'.

Although it's not really a good idea to execute xfstests with SIGPIPE
ignored, this is the only test I've noticed where it causes a problem,
so it might as well be fixed in the test.

It would be much nicer to prevent this problem for all tests by making
the 'check' script restore the default SIGPIPE handler.  But that isn't
straightforward because bash's 'trap' builtin doesn't allow un-ignoring
signals that were ignored on entry to the shell.

Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 tests/generic/397 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Eryu Guan June 13, 2017, 6:54 a.m. UTC | #1
On Mon, Jun 12, 2017 at 02:15:28PM -0700, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
> 
> If generic/397 is executed in an environment with SIGPIPE ignored, it
> fails because the 'yes' program prints an error message:
> 
>     yes: standard output: Broken pipe
>     yes: write error
> 
> This can be reproduced with:
> 
>     trap '' SIGPIPE; ./check generic/397
> 
> Fix it by generating the string of 255 y's using just 'head' and 'tr'
> instead of 'yes', 'head', and 'tr'.
> 
> Although it's not really a good idea to execute xfstests with SIGPIPE
> ignored, this is the only test I've noticed where it causes a problem,
> so it might as well be fixed in the test.

I'm just curious, why do you need to run fstests with SIGPIPE ignored?

> 
> It would be much nicer to prevent this problem for all tests by making
> the 'check' script restore the default SIGPIPE handler.  But that isn't
> straightforward because bash's 'trap' builtin doesn't allow un-ignoring
> signals that were ignored on entry to the shell.
> 
> Signed-off-by: Eric Biggers <ebiggers@google.com>
> ---
>  tests/generic/397 | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/generic/397 b/tests/generic/397
> index 7077d048..ba920891 100755
> --- a/tests/generic/397
> +++ b/tests/generic/397
> @@ -69,7 +69,7 @@ for dir in $SCRATCH_MNT/edir $SCRATCH_MNT/ref_dir; do
>  	touch $dir/empty > /dev/null
>  	$XFS_IO_PROG -t -f -c "pwrite 0 4k" $dir/a > /dev/null
>  	$XFS_IO_PROG -t -f -c "pwrite 0 33k" $dir/abcdefghijklmnopqrstuvwxyz > /dev/null
> -	maxname=$(yes | head -255 | tr -d '\n') # 255 character filename
> +	maxname=$(head -c 255 /dev/zero | tr '\0' y) # 255 character filename

Using perl seems simpler, we have some other tests do similar tasks
using perl too.

	maxname=$($PERL_PROG -e 'print "y" x 255;')

Thanks,
Eryu

>  	$XFS_IO_PROG -t -f -c "pwrite 0 1k" $dir/$maxname > /dev/null
>  	ln -s a $dir/symlink
>  	ln -s abcdefghijklmnopqrstuvwxyz $dir/symlink2
> -- 
> 2.13.1.508.gb3defc5cc-goog
> 
> --
> To unsubscribe from this list: send the line "unsubscribe fstests" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe fstests" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Eric Biggers June 13, 2017, 5:38 p.m. UTC | #2
Hi Eryu,

On Tue, Jun 13, 2017 at 02:54:38PM +0800, Eryu Guan wrote:
> On Mon, Jun 12, 2017 at 02:15:28PM -0700, Eric Biggers wrote:
> > From: Eric Biggers <ebiggers@google.com>
> > 
> > If generic/397 is executed in an environment with SIGPIPE ignored, it
> > fails because the 'yes' program prints an error message:
> > 
> >     yes: standard output: Broken pipe
> >     yes: write error
> > 
> > This can be reproduced with:
> > 
> >     trap '' SIGPIPE; ./check generic/397
> > 
> > Fix it by generating the string of 255 y's using just 'head' and 'tr'
> > instead of 'yes', 'head', and 'tr'.
> > 
> > Although it's not really a good idea to execute xfstests with SIGPIPE
> > ignored, this is the only test I've noticed where it causes a problem,
> > so it might as well be fixed in the test.
> 
> I'm just curious, why do you need to run fstests with SIGPIPE ignored?
> 

It's more of an accidental thing, and I'm likely still going to fix the specific
case I encountered where 'check' was being run with SIGPIPE being ignored
(regardless of whether this xfstests patch goes in or not).  But I think it's an
easy problem for others to run into, since sometimes processes ignore SIGPIPE
because they want to get write errors instead, but then when doing fork() +
exec() they forget to reset the SIGPIPE handler.  Notably, Python got this wrong
and it wasn't fixed until Python 3, so any programs executing the 'check' script
from a Python 2 script will usually get this wrong (see:
https://bugs.python.org/issue1652).  And usually everything works fine but every
once in a while there is a weird problem like this which has to be debugged.

> 
> Using perl seems simpler, we have some other tests do similar tasks
> using perl too.
> 
> 	maxname=$($PERL_PROG -e 'print "y" x 255;')
> 

Yes, the perl version works too.

Eric
--
To unsubscribe from this list: send the line "unsubscribe fstests" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/tests/generic/397 b/tests/generic/397
index 7077d048..ba920891 100755
--- a/tests/generic/397
+++ b/tests/generic/397
@@ -69,7 +69,7 @@  for dir in $SCRATCH_MNT/edir $SCRATCH_MNT/ref_dir; do
 	touch $dir/empty > /dev/null
 	$XFS_IO_PROG -t -f -c "pwrite 0 4k" $dir/a > /dev/null
 	$XFS_IO_PROG -t -f -c "pwrite 0 33k" $dir/abcdefghijklmnopqrstuvwxyz > /dev/null
-	maxname=$(yes | head -255 | tr -d '\n') # 255 character filename
+	maxname=$(head -c 255 /dev/zero | tr '\0' y) # 255 character filename
 	$XFS_IO_PROG -t -f -c "pwrite 0 1k" $dir/$maxname > /dev/null
 	ln -s a $dir/symlink
 	ln -s abcdefghijklmnopqrstuvwxyz $dir/symlink2