On 20/06/18 23:18, Bernhard Voelker wrote:
> On 06/21/2018 05:52 AM, Pádraig Brady wrote:
>> * tests/rm/r-root.sh: gdb provides extra protection,
>> but is not strictly necessary. So provide an option
>> for maintainers to relax the requirements.
>> ---
>> tests/rm/r-root.sh | 36 ++++++++++++++++++++++++------------
>> 1 file changed, 24 insertions(+), 12 deletions(-)
>>
>> diff --git a/tests/rm/r-root.sh b/tests/rm/r-root.sh
>> index c8a57af..14bd0d9 100755
>> --- a/tests/rm/r-root.sh
>> +++ b/tests/rm/r-root.sh
>> @@ -36,12 +36,18 @@ require_gcc_shared_
>> # used later in the unlinkat() wrapper.
>> unset CU_TEST_SKIP_EXIT
>>
>> -# Use gdb to provide further protection by limiting calls to unlinkat().
>> -( timeout 10s gdb --version ) > gdb.out 2>&1
>> -case $(cat gdb.out) in
>> +# Set this to 0 if you don't have a working gdb but would
>> +# still like to run the test
>> +USE_GDB=1
>> +
>> +if test $USE_GDB = 1; then
>> + # Use gdb to provide further protection by limiting calls to unlinkat().
>> + ( timeout 10s gdb --version ) > gdb.out 2>&1
>> + case $(cat gdb.out) in
>> *'GNU gdb'*) ;;
>> *) skip_ "can't run gdb";;
>> -esac
>> + esac
>> +fi
>
> Wouldn't it be even more flexible if the we'd set USE_GDB=0 instead
> of skip_?
Failing back like that would negate the protection by default.
I want to make it an explicit decision.
> BTW: setting USE_GDB=0 unveils:
>
> ./tests/rm/r-root.sh: line 222: test: =: unary operator expected
>
> The variable 'fail' should be initialized:
>
> test $fail = 1 && { cat out; cat err; Exit $fail; }
>
> ... *BUT* it seems that we have more cases like that:
>
> $ grep -F 'unary operator expected' tests/*/*.log | sort -u | wc -l
> 15
>
> Maybe we should initialize 'fail' in a central place?
Hrm maybe. We should at least s/\$fail/"$fail"/.
See also this syntax check which prohibits fail=0 in tests:
https://git.sv.gnu.org/cgit/coreutils.git/commit/?id=v8.0-79-g71c2f88
cheers,
Pádraig
