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.
