[LTP] [PATCH ltp] kernel/fs/doio/rwtest: fix shellcheck error

Petr Vorel pvorel@suse.cz
Mon Jun 25 10:41:20 CEST 2018


Hi Yixin,

thanks for your patch, we appreciate your effort.
There are some comments bellow. Even if you fix them, patches like this one
doesn't help much. The script is really ugly, requires a rewrite which would:

1) Be posix compliant. I.e.: remove all bashisms (arrays, typeset, regex
in [[ ... ]], etc.). It's good to use checkbashism.pl script and run it in dash
to test it:
https://github.com/linux-test-project/ltp/wiki/Test-Writing-Guidelines#132-shell-coding-style
https://salsa.debian.org/debian/devscripts/raw/master/scripts/checkbashisms.pl

2) Use LTP new shell API
https://github.com/linux-test-project/ltp/wiki/Test-Writing-Guidelines#23-writing-a-testcase-in-shell

Rewriting the script would be a great improvement. If you decide to do so,
please do not hesitate to ask for help (if needed).

Kind regards,
Petr

> testcases/kernel/fs/doio/rwtest:199:12: error: > is for string comparisons. Use -gt instead. [SC2071]
> testcases/kernel/fs/doio/rwtest:320:10: error: Globs are ignored in [[ ]] except right of =/!=. Use a loop. [SC2203]
> testcases/kernel/fs/doio/rwtest:347:7: error: Couldn't parse this test expression. [SC1073]
> testcases/kernel/fs/doio/rwtest:347:25: error: Expected test to end here (don't wrap commands in []/[[]]).
>                                                Fix any mentioned problems and try again. [SC1072]

...
> +++ b/testcases/kernel/fs/doio/rwtest
> @@ -196,7 +196,7 @@ do	case $1 in
>  	-n | -Dn )
>  		dOpts="$dOpts $1 $2"
>  		# force file locking with > 1 process
> -		if [[ $2 > 1 ]]
> +		if [[ $2 -gt 1 ]]
Why not use posix compliant syntax? I.e. single square brackets:
if [ $2 -gt 1 ]; then

>  		then
>  			dOpts="$dOpts -k"
>  		fi
> @@ -317,7 +317,7 @@ do
>  		typeset -i n=0
>  		while (( n < ${#szcache[*]} ))
>  		do
> -			if [[ szcache[$n] = $dir ]]; then
> +			if [[ ${szcache[n]} = $dir ]]; then
Well, correct, but we'd like to drop arrays, as they're bashisms.
>  				break;
>  			fi
>  			n=n+1
> @@ -344,7 +344,7 @@ do

>  			# check if blks is a number, else set a default value for blks
>  			default_sz=1000000
> -			if [ $blks -eq $blks 2> /dev/null ]
> +			if [ $blks -eq $blks ] 2>/dev/null
The shellcheck warning was meant to be without square brackets:
if $blks -eq $blks 2>/dev/null; then



More information about the ltp mailing list