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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Thu, 4 Apr 2019 09:24:21 -0600
From:   David Ahern <dsahern@...il.com>
To:     Stefano Brivio <sbrivio@...hat.com>,
        David Ahern <dsahern@...nel.org>
Cc:     davem@...emloft.net, netdev@...r.kernel.org
Subject: Re: [PATCH net-next] selftests: Add debugging options to pmtu.sh

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.