Re: [PATCH] tail: fix a race condition

[Jim Meyering]

Giuseppe Scrivano wrote:
> +++ b/tests/tail-2/inotify-race

Thanks.
With something like the following it should be good.

> @@ -0,0 +1,72 @@
> +#!/bin/sh
> +# Copyright (C) 2006, 2009 Free Software Foundation, Inc.

It's customary to say what the test does.

# Ensure that tail does not ignore data that is appended to a tailed-forever
# file between tail's initial read-to-EOF, and when the inotify watches
# are established in tail_forever_inotify.  That data could be ignored
# indefinitely if no *other* data is appended, but it would be printed as
# soon as any additional appended data is detected.

> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; either version 2 of the License, or
> +# (at your option) any later version.
> +
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +
> +# You should have received a copy of the GNU General Public License
> +# along with this program; if not, write to the Free Software
> +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA
> +# 02110-1301, USA.
> +
> +if test "$VERBOSE" = yes; then
> +  set -x
> +  tail --version
> +fi
> +
> +. $srcdir/test-lib.sh
> +
> +fail=0
> +
> +umask 2

No need to set umask -- was relevant to the old test,
not this one.

> +touch file || framework_failure
> +
> +( gdb --version ) > gdb.out 2>&1
> +case $(cat gdb.out) in
> +  'GNU gdb'*) ;;
> +  *) echo "$0: can't run gdb.  Skipping this test." 1>&2;
> +     (exit 77); exit 77;;

Please modernize this:
(I'll add a syntax-check to catch these from now on)

     *) skip_test_ "can't run gdb";;

> +esac
> +
> +
> +# Use `xnanosleep' to sleep the debugged process for a specified
> +# amount of time.  `xnanosleep' is used by tail and the symbol is
> +# exported, so we can use it without problems.

xnanosleep is no longer relevant, so you can remove the above comment.
How about just:

# See if gdb works:

> +gdb -nx --batch-silent              \
> +    --eval-command='break tail_forever_inotify'    \
> +    --eval-command='run -f file'    \
> +    --eval-command='quit'    \
> +    $abs_top_builddir/src/tail < /dev/null > gdb.out 2>&1
> +
> +if test -s gdb.out; then
> +  cat <<EOF 1>&2
> +$0: can't set breakpoints in tail.  Skipping this test.
> +EOF
> +  exit 77

You can use skip_test_ here, too:

   test -s gdb.out \
     && skip_test_ "can't set breakpoints in tail"

> +gdb -nx --batch-silent              \
> +    --eval-command='break tail_forever_inotify'    \
> +    --eval-command='run -f file'    \

redirect to new file (used below):
       --eval-command='run -f file > tail.out'    \

> +    --eval-command="shell echo never-seen-with-tail-7.5 >> file" \
> +    --eval-command='continue' \
> +    --eval-command='quit'    \
> +    $abs_top_builddir/src/tail < /dev/null > gdb.out &
> +
> +pid=$!
> +
> +sleep 1.5s
> +
> +kill $pid

You might want to invoke gdb via "timeout".
Then you can remove both the sleep and the kill.

Use a long timeout (say 10 seconds), but when you see that appended
line appear in tail -f's output, you can kill the process early.
It's only with the buggy tail (the timeout expires without the
line appearing in tail's output) that the test will take 10 seconds.

> +test -s gdb.out || fail=1
> +
> +Exit $fail