Re: [PATCH net-next] selftests: Add debugging options to pmtu.sh

[David Ahern <dsahern@gmail.com>]

On 4/4/19 6:16 AM, Stefano Brivio wrote:
>> diff --git a/tools/testing/selftests/net/pmtu.sh b/tools/testing/selftests/net/pmtu.sh
>> index 912b2dc50be3..28e8c97b5c9e 100755
>> --- a/tools/testing/selftests/net/pmtu.sh
>> +++ b/tools/testing/selftests/net/pmtu.sh
>> @@ -116,6 +116,9 @@
>>  # Kselftest framework requirement - SKIP code is 4.
>>  ksft_skip=4
>>  
>> +PAUSE_ON_FAIL=no
>> +VERBOSE=0
> 
> For consistency, I'd also rename 'tracing' below to TRACING and assign
> it here.

ok.

> 
>>  # Some systems don't have a ping6 binary anymore
>>  which ping6 > /dev/null 2>&1 && ping6=$(which ping6) || ping6=$(which ping)
>>  
>> @@ -222,6 +225,26 @@ err_flush() {
>>  	err_buf=
>>  }
>>  
>> +run_cmd() {
>> +	local cmd="$*"
>> +	local out
>> +	local stderr="2>/dev/null"
> 
> 'local' is not POSIX, and I think it actually breaks (at least) on
> ksh93 (maybe not a big deal, but I kept everything else POSIX, so I
> wouldn't break it just for this).

ok. I did not realize this one is expected to be posix compliant. I have
fib_test.sh using /bin/bash

> 
> Besides, for 'ping' commands, it's stdout that needs to be suppressed
> (we can just suppress both stdout and stderr if not in verbose mode).

sure

> 
>> +	if [ "$VERBOSE" = "1" ]; then
>> +		printf "    COMMAND: $cmd\n"
>> +		stderr=
>> +	fi
>> +
>> +	out=$(eval $cmd $stderr)
> 
> I think this needs quoting. Is eval really needed, by the way?

old habits. Works fine with bash; not sure about others.