Re: [PATCH] common/rc: add missing 'local' keywords

[Eryu Guan <guaneryu@gmail.com>]

On Thu, Apr 05, 2018 at 11:31:29AM -0700, Eric Biggers wrote:
> A lot of the helper functions in xfstests are unnecessarily declaring
> variables without the 'local' keyword, which pollutes the global
> namespace and can collide with variables in tests.  Fix this for
> everything in common/rc that I could find.
> 
> Signed-off-by: Eric Biggers <ebiggers@google.com>

Thanks a lot for doing this! I need some time to do careful testing, as
something can be broken implicitly, as the "$err_msg" usage below.

> ---
>  common/rc | 306 ++++++++++++++++++++++++++++--------------------------
>  1 file changed, 158 insertions(+), 148 deletions(-)
> 
> diff --git a/common/rc b/common/rc
> index 6a91850c..39e1db43 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -53,12 +53,8 @@ _require_math()
>  _math()
>  {
>  	[ $# -le 0 ] && return
> -	if [ "$BC" ]; then
> -		result=$(LANG=C echo "scale=0; $@" | "$BC" -q 2> /dev/null)
> -	else
> -		_notrun "this test requires 'bc' tool for doing math operations"
> -	fi
> -	echo "$result"
> +	_require_math

I think _require_math belongs to tests that take use of _math, not _math
itself.

> +	LANG=C echo "scale=0; $@" | "$BC" -q 2> /dev/null
>  }
>  
>  dd()
> @@ -86,22 +82,22 @@ _md5_checksum()
>  
>  # Write a byte into a range of a file
>  _pwrite_byte() {
> -	pattern="$1"
> -	offset="$2"
> -	len="$3"
> -	file="$4"
> -	xfs_io_args="$5"
> +	local pattern="$1"
> +	local offset="$2"
> +	local len="$3"
> +	local file="$4"
> +	local xfs_io_args="$5"
>  
>  	$XFS_IO_PROG $xfs_io_args -f -c "pwrite -S $pattern $offset $len" "$file"
>  }
>  
>  # mmap-write a byte into a range of a file
>  _mwrite_byte() {
> -	pattern="$1"
> -	offset="$2"
> -	len="$3"
> -	mmap_len="$4"
> -	file="$5"
> +	local pattern="$1"
> +	local offset="$2"
> +	local len="$3"
> +	local mmap_len="$4"
> +	local file="$5"
>  
>  	$XFS_IO_PROG -f -c "mmap -rw 0 $mmap_len" -c "mwrite -S $pattern $offset $len" "$file"
>  }
> @@ -127,19 +123,19 @@ fi
>  
>  _dump_err()
>  {
> -    err_msg="$*"
> +    local err_msg="$*"
>      echo "$err_msg"
>  }
>  
>  _dump_err2()
>  {
> -    err_msg="$*"
> +    local err_msg="$*"
>      >2& echo "$err_msg"
>  }
>  
>  _log_err()
>  {
> -    err_msg="$*"
> +    local err_msg="$*"

It seems the "$err_msg" in above functions need to be global,
common/report needs it set somewhere. Perhaps we can name it with a
leading underscore to indicate it's a global variable.

>      echo "$err_msg" | tee -a $seqres.full
>      echo "(see $seqres.full for details)"
>  }
> @@ -249,6 +245,8 @@ _clear_mount_stack()
>  _scratch_options()
>  {
>      local type=$1
> +    local rt_opt=""
> +    local log_opt=""
>      SCRATCH_OPTIONS=""
>  
>      if [ "$FSTYP" != "xfs" ]; then
> @@ -274,7 +272,9 @@ _scratch_options()
>  
>  _test_options()
>  {
> -    type=$1
> +    local type=$1
> +    local rt_opt=""
> +    local log_opt=""
>      TEST_OPTIONS=""
>  
>      if [ "$FSTYP" != "xfs" ]; then
> @@ -299,15 +299,15 @@ _test_options()
>  
>  _mount_ops_filter()
>  {
> -    params="$*"
> -    
> +    local params="$*"
> +    local last_index=$(( $# - 1 ))
> +
>      #get mount point to handle dmapi mtpt option correctly
> -    let last_index=$#-1
>      [ $last_index -gt 0 ] && shift $last_index
> -    FS_ESCAPED=$1
> -    
> +    local fs_escaped=$1
> +
>      echo $params | sed -e 's/dmapi/dmi/' \
> -        | $PERL_PROG -ne "s#mtpt=[^,|^\n|^\s]*#mtpt=$FS_ESCAPED\1\2#; print;"
> +        | $PERL_PROG -ne "s#mtpt=[^,|^\n|^\s]*#mtpt=$fs_escaped\1\2#; print;"
>  
>  }
>  
> @@ -506,9 +506,9 @@ _scratch_do_mkfs()
>  
>  _scratch_metadump()
>  {
> -	dumpfile=$1
> +	local dumpfile=$1
>  	shift
> -	options=
> +	local options=
>  
>  	[ "$USE_EXTERNAL" = yes -a ! -z "$SCRATCH_LOGDEV" ] && \
>  		options="-l $SCRATCH_LOGDEV"
> @@ -518,7 +518,7 @@ _scratch_metadump()
>  
>  _setup_large_ext4_fs()
>  {
> -	fs_size=$1
> +	local fs_size=$1
>  	local tmp_dir=/tmp/
>  
>  	[ "$LARGE_SCRATCH_DEV" != yes ] && return 0
> @@ -527,7 +527,7 @@ _setup_large_ext4_fs()
>  
>  	# Default free space in the FS is 50GB, but you can specify more via
>  	# SCRATCH_DEV_EMPTY_SPACE
> -	space_to_consume=$(($fs_size - 50*1024*1024*1024 - $SCRATCH_DEV_EMPTY_SPACE))
> +	local space_to_consume=$(($fs_size - 50*1024*1024*1024 - $SCRATCH_DEV_EMPTY_SPACE))
>  
>  	# mount the filesystem and create 16TB - 4KB files until we consume
>  	# all the necessary space.
> @@ -541,8 +541,8 @@ _setup_large_ext4_fs()
>  	fi
>  	rm -f $tmp_dir/mnt.err
>  
> -	file_size=$((16*1024*1024*1024*1024 - 4096))
> -	nfiles=0
> +	local file_size=$((16*1024*1024*1024*1024 - 4096))
> +	local nfiles=0
>  	while [ $space_to_consume -gt $file_size ]; do
>  
>  		xfs_io -F -f \
> @@ -593,7 +593,7 @@ _scratch_mkfs_ext4()
>  
>  	if [ $mkfs_status -eq 0 -a "$LARGE_SCRATCH_DEV" = yes ]; then
>  		# manually parse the mkfs output to get the fs size in bytes
> -		fs_size=`cat $tmp.mkfsstd | awk ' \
> +		local fs_size=`cat $tmp.mkfsstd | awk ' \
>  			/^Block size/ { split($2, a, "="); bs = a[2] ; } \
>  			/ inodes, / { blks = $3 } \
>  			/reserved for the super user/ { resv = $1 } \
> @@ -929,8 +929,9 @@ _free_memory_bytes()
>  # _scratch_mkfs_sized <size in bytes> [optional blocksize]
>  _scratch_mkfs_sized()
>  {
> -    fssize=$1
> -    blocksize=$2
> +    local fssize=$1
> +    local blocksize=$2
> +    local def_blksz
>  
>      case $FSTYP in
>      xfs)
> @@ -948,7 +949,7 @@ _scratch_mkfs_sized()
>      [ -z "$blocksize" ] && blocksize=4096
>  
>  
> -    re='^[0-9]+$'
> +    local re='^[0-9]+$'
>      if ! [[ $fssize =~ $re ]] ; then
>          _notrun "error: _scratch_mkfs_sized: fs size \"$fssize\" not an integer."
>      fi
> @@ -956,10 +957,10 @@ _scratch_mkfs_sized()
>          _notrun "error: _scratch_mkfs_sized: block size \"$blocksize\" not an integer."
>      fi
>  
> -    blocks=`expr $fssize / $blocksize`
> +    local blocks=`expr $fssize / $blocksize`
>  
>      if [ "$HOSTOS" == "Linux" -a -b "$SCRATCH_DEV" ]; then
> -	devsize=`blockdev --getsize64 $SCRATCH_DEV`
> +	local devsize=`blockdev --getsize64 $SCRATCH_DEV`
>  	[ "$fssize" -gt "$devsize" ] && _notrun "Scratch device too small"
>      fi
>  
> @@ -982,12 +983,12 @@ _scratch_mkfs_sized()
>  	# filesystem, which will cause mkfs.gfs2 to fail.  Until that's fixed,
>  	# shrink the journal size to at most one eigth of the filesystem and at
>  	# least 8 MiB, the minimum size allowed.
> -	MIN_JOURNAL_SIZE=8
> -	DEFAULT_JOURNAL_SIZE=128
> -	if (( fssize/8 / (1024*1024) < DEFAULT_JOURNAL_SIZE )); then
> -	    (( JOURNAL_SIZE = fssize/8 / (1024*1024) ))
> -	    (( JOURNAL_SIZE >= MIN_JOURNAL_SIZE )) || JOURNAL_SIZE=$MIN_JOURNAL_SIZE
> -	    MKFS_OPTIONS="-J $JOURNAL_SIZE $MKFS_OPTIONS"
> +	local min_journal_size=8
> +	local default_journal_size=128
> +	if (( fssize/8 / (1024*1024) < default_journal_size )); then
> +	    local journal_size=$(( fssize/8 / (1024*1024) ))
> +	    (( journal_size >= min_journal_size )) || journal_size=$min_journal_size
> +	    MKFS_OPTIONS="-J $journal_size $MKFS_OPTIONS"
>  	fi
>  	${MKFS_PROG}.$FSTYP $MKFS_OPTIONS -O -b $blocksize $SCRATCH_DEV $blocks
>  	;;
> @@ -1015,11 +1016,11 @@ _scratch_mkfs_sized()
>  	;;
>      f2fs)
>  	# mkfs.f2fs requires # of sectors as an input for the size
> -	sector_size=`blockdev --getss $SCRATCH_DEV`
> +	local sector_size=`blockdev --getss $SCRATCH_DEV`
>  	$MKFS_F2FS_PROG $MKFS_OPTIONS $SCRATCH_DEV `expr $fssize / $sector_size`
>  	;;
>      tmpfs)
> -	free_mem=`_free_memory_bytes`
> +	local free_mem=`_free_memory_bytes`
>  	if [ "$free_mem" -lt "$fssize" ] ; then
>  	   _notrun "Not enough memory ($free_mem) for tmpfs with $fssize bytes"
>  	fi
> @@ -1035,13 +1036,13 @@ _scratch_mkfs_sized()
>  # _scratch_mkfs_geom <sunit bytes> <swidth multiplier> [optional blocksize]
>  _scratch_mkfs_geom()
>  {
> -    sunit_bytes=$1
> -    swidth_mult=$2
> -    blocksize=$3
> +    local sunit_bytes=$1
> +    local swidth_mult=$2
> +    local blocksize=$3
>      [ -z "$blocksize" ] && blocksize=4096
>  
> -    let sunit_blocks=$sunit_bytes/$blocksize
> -    let swidth_blocks=$sunit_blocks*$swidth_mult
> +    local sunit_blocks=$(( sunit_bytes / blocksize ))
> +    local swidth_blocks=$(( sunit_blocks * swidth_mult ))
>  
>      case $FSTYP in
>      xfs)
> @@ -1061,9 +1062,9 @@ _scratch_mkfs_geom()
>  # _scratch_mkfs_blocksized blocksize
>  _scratch_mkfs_blocksized()
>  {
> -    blocksize=$1
> +    local blocksize=$1
>  
> -    re='^[0-9]+$'
> +    local re='^[0-9]+$'
>      if ! [[ $blocksize =~ $re ]] ; then
>          _notrun "error: _scratch_mkfs_sized: block size \"$blocksize\" not an integer."
>      fi
> @@ -1107,7 +1108,7 @@ _repair_scratch_fs()
>      case $FSTYP in
>      xfs)
>          _scratch_xfs_repair "$@" 2>&1
> -	res=$?
> +	local res=$?
>  	if [ "$res" -ne 0 ]; then
>  		echo "xfs_repair returns $res; replay log?"
>  		_try_scratch_mount
> @@ -1130,7 +1131,7 @@ _repair_scratch_fs()
>      *)
>          # Let's hope fsck -y suffices...
>          fsck -t $FSTYP -y $SCRATCH_DEV 2>&1
> -	res=$?
> +	local res=$?
>  	case $res in
>  	0|1|2)
>  		res=0
> @@ -1317,7 +1318,7 @@ _is_block_dev()
>  	exit 1
>      fi
>  
> -    _dev=$1
> +    local _dev=$1

A 'local' variable could drop the leading underscore then.

>      if [ -L "${_dev}" ]; then
>          _dev=`readlink -f "${_dev}"`
>      fi
> @@ -1336,7 +1337,7 @@ _is_char_dev()
>  		exit 1
>  	fi
>  
> -	_dev=$1
> +	local _dev=$1

Same here.

>  	if [ -L "${_dev}" ]; then
>  		_dev=`readlink -f "${_dev}"`
>  	fi
> @@ -1359,10 +1360,10 @@ _is_char_dev()
>  _do()
>  {
>      if [ $# -eq 1 ]; then
> -	_cmd=$1
> +	local _cmd=$1
>      elif [ $# -eq 2 ]; then
> -	_note=$1
> -	_cmd=$2
> +	local _note=$1
> +	local _cmd=$2

Same here.

>  	echo -n "$_note... "
>      else
>  	echo "Usage: _do [note] cmd" 1>&2
> @@ -1370,7 +1371,8 @@ _do()
>      fi
>  
>      (eval "echo '---' \"$_cmd\"") >>$seqres.full
> -    (eval "$_cmd") >$tmp._out 2>&1; ret=$?
> +    (eval "$_cmd") >$tmp._out 2>&1
> +    local ret=$?
>      cat $tmp._out | _fix_malloc >>$seqres.full
>      rm -f $tmp._out
>      if [ $# -eq 2 ]; then
> @@ -1420,6 +1422,8 @@ _fail()
>  #
>  _supported_fs()
>  {
> +    local f
> +
>      for f
>      do
>  	if [ "$f" = "$FSTYP" -o "$f" = "generic" ]
> @@ -1436,6 +1440,8 @@ _supported_fs()
>  #
>  _supported_os()
>  {
> +    local h
> +
>      for h
>      do
>  	if [ "$h" = "$HOSTOS" ]
> @@ -1800,14 +1806,14 @@ _require_no_realtime()
>  _require_command()
>  {
>  	if [ $# -eq 2 ]; then
> -		_name="$2"
> +		local _name="$2"
>  	elif [ $# -eq 1 ]; then
> -		_name="$1"
> +		local _name="$1"
>  	else
>  		_fail "usage: _require_command <command> [<name>]"
>  	fi
>  
> -	_command=`echo "$1" | awk '{ print $1 }'`
> +	local _command=`echo "$1" | awk '{ print $1 }'`

Same here.

>  	if [ ! -x "$_command" ]; then
>  		_notrun "$_name utility required, skipped this test"
>  	fi
> @@ -1857,7 +1863,7 @@ _require_sane_bdev_flush()
>  # this test requires a specific device mapper target
>  _require_dm_target()
>  {
> -	_target=$1
> +	local _target=$1

Same here.

I'll test other changes on different $FSTYP.

Thanks,
Eryu

>  
>  	# require SCRATCH_DEV to be a valid block device with sane BLKFLSBUF
>  	# behaviour
> @@ -1947,8 +1953,8 @@ _require_aiodio()
>  #
>  _require_test_program()
>  {
> -    SRC_TEST=src/$1
> -    [ -x $SRC_TEST ] || _notrun "$SRC_TEST not built"
> +    local prog=src/$1
> +    [ -x $prog ] || _notrun "$prog not built"
>  }
>  
>  # run an aio-dio program
> @@ -1992,7 +1998,7 @@ _require_y2038()
>  
>  _filesystem_timestamp_range()
>  {
> -	device=${1:-$TEST_DEV}
> +	local device=${1:-$TEST_DEV}
>  	case $FSTYP in
>  	ext4)
>  		if [ $(dumpe2fs -h $device 2>/dev/null | grep "Inode size:" | cut -d: -f2) -gt 128 ]; then
> @@ -2097,13 +2103,14 @@ _require_xfs_io_command()
>  	local param_checked=0
>  	local opts=""
>  
> -	testfile=$TEST_DIR/$$.xfs_io
> +	local testfile=$TEST_DIR/$$.xfs_io
> +	local testio
>  	case $command in
>  	"chproj")
>  		testio=`$XFS_IO_PROG -F -f -c "chproj 0" $testfile 2>&1`
>  		;;
>  	"copy_range")
> -		testcopy=$TEST_DIR/$$.copy.xfs_io
> +		local testcopy=$TEST_DIR/$$.copy.xfs_io
>  		$XFS_IO_PROG -F -f -c "pwrite 0 4k" $testfile > /dev/null 2>&1
>  		testio=`$XFS_IO_PROG -F -f -c "copy_range $testfile" $testcopy 2>&1`
>  		rm -f $testcopy > /dev/null 2>&1
> @@ -2211,7 +2218,7 @@ _require_odirect()
>  			_notrun "ext4 data journaling doesn't support O_DIRECT"
>  		fi
>  	fi
> -	testfile=$TEST_DIR/$$.direct
> +	local testfile=$TEST_DIR/$$.direct
>  	$XFS_IO_PROG -F -f -d -c "pwrite 0 20k" $testfile > /dev/null 2>&1
>  	if [ $? -ne 0 ]; then
>  		_notrun "O_DIRECT is not supported"
> @@ -2245,13 +2252,13 @@ _require_scratch_swapfile()
>  #
>  _require_fs_space()
>  {
> -	MNT=$1
> -	BLOCKS=$2	# in units of 1024
> -	let GB=$BLOCKS/1024/1024
> +	local mnt=$1
> +	local blocks=$2	# in units of 1024
> +	local gb=$(( blocks / 1024 / 1024 ))
>  
> -	FREE_BLOCKS=`df -kP $MNT | grep -v Filesystem | awk '{print $4}'`
> -	[ $FREE_BLOCKS -lt $BLOCKS ] && \
> -		_notrun "This test requires at least ${GB}GB free on $MNT to run"
> +	local free_blocks=`df -kP $mnt | grep -v Filesystem | awk '{print $4}'`
> +	[ $free_blocks -lt $blocks ] && \
> +		_notrun "This test requires at least ${gb}GB free on $mnt to run"
>  }
>  
>  #
> @@ -2418,8 +2425,8 @@ _remount()
>  	echo "Usage: _remount device ro/rw" 1>&2
>  	exit 1
>      fi
> -    device=$1
> -    mode=$2
> +    local device=$1
> +    local mode=$2
>  
>      if ! mount -o remount,$mode $device
>      then
> @@ -2445,8 +2452,8 @@ _umount_or_remount_ro()
>  	exit 1
>      fi
>  
> -    device=$1
> -    mountpoint=`_is_dev_mounted $device`
> +    local device=$1
> +    local mountpoint=`_is_dev_mounted $device`
>  
>      if [ $USE_REMOUNT -eq 0 ]; then
>          $UMOUNT_PROG $device
> @@ -2462,9 +2469,9 @@ _mount_or_remount_rw()
>  		echo "Usage: _mount_or_remount_rw <opts> <dev> <mnt>" 1>&2
>  		exit 1
>  	fi
> -	mount_opts=$1
> -	device=$2
> -	mountpoint=$3
> +	local mount_opts=$1
> +	local device=$2
> +	local mountpoint=$3
>  
>  	if [ $USE_REMOUNT -eq 0 ]; then
>  		if [ "$FSTYP" != "overlay" ]; then
> @@ -2492,16 +2499,16 @@ _mount_or_remount_rw()
>  #
>  _check_generic_filesystem()
>  {
> -    device=$1
> +    local device=$1
>  
>      # If type is set, we're mounted
> -    type=`_fs_type $device`
> -    ok=1
> +    local type=`_fs_type $device`
> +    local ok=1
>  
>      if [ "$type" = "$FSTYP" ]
>      then
>          # mounted ...
> -        mountpoint=`_umount_or_remount_ro $device`
> +        local mountpoint=`_umount_or_remount_ro $device`
>      fi
>  
>      fsck -t $FSTYP $FSCK_OPTIONS $device >$tmp.fsck 2>&1
> @@ -2566,16 +2573,15 @@ _check_udf_filesystem()
>  	return
>      fi
>  
> -    device=$1
> -    if [ $# -eq 2 ];
> -    then
> -        LAST_BLOCK=`expr \( $2 - 1 \)`
> -        OPT_ARG="-lastvalidblock $LAST_BLOCK"
> +    local device=$1
> +    local opt_arg=""
> +    if [ $# -eq 2 ]; then
> +	opt_arg="-lastvalidblock $(( $2 - 1 ))"
>      fi
>  
>      rm -f $seqres.checkfs
>      sleep 1 # Due to a problem with time stamps in udf_test
> -    $here/src/udf_test $OPT_ARG $device | tee $seqres.checkfs | egrep "Error|Warning" | \
> +    $here/src/udf_test $opt_arg $device | tee $seqres.checkfs | egrep "Error|Warning" | \
>  	_udf_test_known_error_filter | \
>  	egrep -iv "Error count:.*[0-9]+.*total occurrences:.*[0-9]+|Warning count:.*[0-9]+.*total occurrences:.*[0-9]+" && \
>          echo "Warning UDF Verifier reported errors see $seqres.checkfs." && return 1
> @@ -2628,20 +2634,20 @@ _check_test_fs()
>  
>  _check_scratch_fs()
>  {
> -    device=$SCRATCH_DEV
> +    local device=$SCRATCH_DEV
>      [ $# -eq 1 ] && device=$1
>  
>      case $FSTYP in
>      xfs)
> -	SCRATCH_LOG="none"
> -	SCRATCH_RT="none"
> +	local scratch_log="none"
> +	local scratch_rt="none"
>  	[ "$USE_EXTERNAL" = yes -a ! -z "$SCRATCH_LOGDEV" ] && \
> -	    SCRATCH_LOG="$SCRATCH_LOGDEV"
> +	    scratch_log="$SCRATCH_LOGDEV"
>  
>  	[ "$USE_EXTERNAL" = yes -a ! -z "$SCRATCH_RTDEV" ] && \
> -	    SCRATCH_RT="$SCRATCH_RTDEV"
> +	    scratch_rt="$SCRATCH_RTDEV"
>  
> -	_check_xfs_filesystem $device $SCRATCH_LOG $SCRATCH_RT
> +	_check_xfs_filesystem $device $scratch_log $scratch_rt
>  	;;
>      udf)
>  	_check_udf_filesystem $device $udf_fsize
> @@ -2704,10 +2710,10 @@ _full_fstyp_details()
>  
>  _full_platform_details()
>  {
> -     os=`uname -s`
> -     host=`hostname -s`
> -     kernel=`uname -r`
> -     platform=`uname -m`
> +     local os=`uname -s`
> +     local host=`hostname -s`
> +     local kernel=`uname -r`
> +     local platform=`uname -m`
>       echo "$os/$platform $host $kernel"
>  }
>  
> @@ -2723,8 +2729,8 @@ _get_os_name()
>  
>  _link_out_file_named()
>  {
> -	export FEATURES=$2
> -	SUFFIX=$(perl -e '
> +	local features=$2
> +	local suffix=$(FEATURES="$features" perl -e '
>  		my %feathash;
>  		my $feature, $result, $suffix, $opts;
>  
> @@ -2751,22 +2757,23 @@ _link_out_file_named()
>  		print $result
>  		' <$seqfull.cfg)
>  	rm -f $1
> -	SRC=$(basename $1)
> -	ln -fs $SRC.$SUFFIX $1
> +	ln -fs $(basename $1).$suffix $1
>  }
>  
>  _link_out_file()
>  {
> +	local features
> +
>  	if [ $# -eq 0 ]; then
> -		FEATURES="$(_get_os_name)"
> +		features="$(_get_os_name)"
>  		if [ -n "$MOUNT_OPTIONS" ]; then
> -			FEATURES=$FEATURES,${MOUNT_OPTIONS##"-o "}
> +			features=$features,${MOUNT_OPTIONS##"-o "}
>  		fi
>  	else
> -		FEATURES=$1
> +		features=$1
>  	fi
>  
> -	_link_out_file_named $seqfull.out "$FEATURES"
> +	_link_out_file_named $seqfull.out "$features"
>  }
>  
>  _die()
> @@ -2784,10 +2791,10 @@ _ddt()
>  #takes files, randomdata
>  _nfiles()
>  {
> -        f=0
> +        local f=0
>          while [ $f -lt $1 ]
>          do
> -                file=f$f
> +                local file=f$f
>                  echo > $file
>                  if [ $size -gt 0 ]; then
>                      if [ "$2" == "false" ]; then
> @@ -2805,18 +2812,18 @@ _nfiles()
>  # takes dirname, depth, randomdata
>  _descend()
>  {
> -        dirname=$1; depth=$2; randomdata=$3
> +        local dirname=$1 depth=$2 randomdata=$3
>          mkdir $dirname  || die "mkdir $dirname failed"
>          cd $dirname
>  
>          _nfiles $files $randomdata          # files for this dir and data type
>  
>          [ $depth -eq 0 ] && return
> -	let deep=$depth-1 # go 1 down
> +        local deep=$(( depth - 1 )) # go 1 down
>  
>          [ $verbose = true ] && echo "descending, depth from leaves = $deep"
>  
> -        d=0
> +        local d=0
>          while [ $d -lt $dirs ]
>          do
>                  _descend d$d $deep &
> @@ -2831,14 +2838,15 @@ _descend()
>  #
>  _populate_fs()
>  {
> -    here=`pwd`
> -    dirs=5          # ndirs in each subdir till leaves
> -    size=0          # sizeof files in K
> -    files=100       # num files in _each_ subdir
> -    depth=2         # depth of tree from root to leaves
> -    verbose=false
> -    root=root       # path of initial root of directory tree
> -    randomdata=false # -x data type urandom, zero or compressible
> +    local here=`pwd`
> +    local dirs=5          # ndirs in each subdir till leaves
> +    local size=0          # sizeof files in K
> +    local files=100       # num files in _each_ subdir
> +    local depth=2         # depth of tree from root to leaves
> +    local verbose=false
> +    local root=root       # path of initial root of directory tree
> +    local randomdata=false # -x data type urandom, zero or compressible
> +    local c
>  
>      OPTIND=1
>      while getopts "d:f:n:r:s:v:x:c" c
> @@ -2867,8 +2875,8 @@ _populate_fs()
>  #
>  _test_inode_flag()
>  {
> -	flag=$1
> -	file=$2
> +	local flag=$1
> +	local file=$2
>  
>  	if $XFS_IO_PROG -r -c 'lsattr -v' "$file" | grep -q "$flag" ; then
>  		return 0
> @@ -2880,8 +2888,8 @@ _test_inode_flag()
>  #
>  _test_inode_extsz()
>  {
> -	file=$1
> -	blocks=""
> +	local file=$1
> +	local blocks=""
>  
>  	blocks=`$XFS_IO_PROG -r -c 'stat' "$file" | \
>  		awk '/^xattr.extsize =/ { print $3 }'`
> @@ -2971,7 +2979,7 @@ _require_deletable_scratch_dev_pool()
>  # Check that fio is present, and it is able to execute given jobfile
>  _require_fio()
>  {
> -	job=$1
> +	local job=$1
>  
>  	_require_command "$FIO_PROG" fio
>  	if [ -z "$1" ]; then
> @@ -2986,7 +2994,7 @@ _require_fio()
>  _require_freeze()
>  {
>  	xfs_freeze -f "$TEST_DIR" >/dev/null 2>&1
> -	result=$? 
> +	local result=$?
>  	xfs_freeze -u "$TEST_DIR" >/dev/null 2>&1
>  	[ $result -eq 0 ] || _notrun "$FSTYP does not support freezing"
>  }
> @@ -3061,9 +3069,9 @@ _require_norecovery()
>  _require_metadata_journaling()
>  {
>  	if [ -z $1 ]; then
> -		DEV=$TEST_DEV
> +		local dev=$TEST_DEV
>  	else
> -		DEV=$1
> +		local dev=$1
>  	fi
>  
>  	case "$FSTYP" in
> @@ -3073,8 +3081,8 @@ _require_metadata_journaling()
>  	ext4)
>  		# ext4 could be mkfs'd without a journal...
>  		_require_dumpe2fs
> -		$DUMPE2FS_PROG -h $DEV 2>&1 | grep -q has_journal || \
> -			_notrun "$FSTYP on $DEV not configured with metadata journaling"
> +		$DUMPE2FS_PROG -h $dev 2>&1 | grep -q has_journal || \
> +			_notrun "$FSTYP on $dev not configured with metadata journaling"
>  		# ext4 might not load a journal
>  		_exclude_scratch_mount_option "noload"
>  		;;
> @@ -3140,10 +3148,12 @@ _devmgt_add()
>  	echo ${tdl} >  /sys/class/scsi_host/host${h}/scan || _fail "Add disk failed"
>  
>  	# ensure the device comes online
> -	dev_back_oneline=0
> +	local dev_back_oneline=0
> +	local i
>  	for i in `seq 1 10`; do
>  		if [ -d /sys/class/scsi_device/${1}/device/block ]; then
> -			dev=`ls /sys/class/scsi_device/${1}/device/block`
> +			local dev=`ls /sys/class/scsi_device/${1}/device/block`
> +			local j
>  			for j in `seq 1 10`;
>  			do
>  				stat /dev/$dev > /dev/null 2>&1
> @@ -3257,20 +3267,20 @@ _require_userns()
>  
>  _create_loop_device()
>  {
> -	file=$1
> +	local file=$1 dev
>  	dev=`losetup -f --show $file` || _fail "Cannot assign $file to a loop device"
>  	echo $dev
>  }
>  
>  _destroy_loop_device()
>  {
> -	dev=$1
> +	local dev=$1
>  	losetup -d $dev || _fail "Cannot destroy loop device $dev"
>  }
>  
>  _scale_fsstress_args()
>  {
> -    args=""
> +    local args=""
>      while [ $# -gt 0 ]; do
>          case "$1" in
>              -n) args="$args $1 $(($2 * $TIME_FACTOR))"; shift ;;
> @@ -3288,7 +3298,7 @@ _scale_fsstress_args()
>  #
>  _min_dio_alignment()
>  {
> -    dev=$1
> +    local dev=$1
>  
>      if [ -b "$dev" ]; then
>          blockdev --getss $dev
> @@ -3305,8 +3315,8 @@ run_check()
>  
>  _require_test_symlinks()
>  {
> -	target=`mktemp -p $TEST_DIR`
> -	link=`mktemp -p $TEST_DIR -u`
> +	local target=`mktemp -p $TEST_DIR`
> +	local link=`mktemp -p $TEST_DIR -u`
>  	ln -s `basename $target` $link
>  	if [ "$?" -ne 0 ]; then
>  		rm -f $target
> @@ -3336,7 +3346,7 @@ _require_ofd_locks()
>  
>  _require_test_lsattr()
>  {
> -	testio=$(lsattr -d $TEST_DIR 2>&1)
> +	local testio=$(lsattr -d $TEST_DIR 2>&1)
>  	echo $testio | grep -q "Operation not supported" && \
>  		_notrun "lsattr not supported by test filesystem type: $FSTYP"
>  	echo $testio | grep -q "Inappropriate ioctl for device" && \
> @@ -3353,9 +3363,9 @@ _require_chattr()
>  
>  	touch $TEST_DIR/syscalltest
>  	chattr "+$attribute" $TEST_DIR/syscalltest > $TEST_DIR/syscalltest.out 2>&1
> -	status=$?
> +	local ret=$?
>  	chattr "-$attribute" $TEST_DIR/syscalltest > $TEST_DIR/syscalltest.out 2>&1
> -	if [ "$status" -ne 0 ]; then
> +	if [ "$ret" -ne 0 ]; then
>  		_notrun "file system doesn't support chattr +$attribute"
>  	fi
>  	cat $TEST_DIR/syscalltest.out >> $seqres.full
> @@ -3462,7 +3472,7 @@ _check_dmesg()
>  	# default filter is a simple cat command, caller could provide a
>  	# customized filter and pass the name through the first argument, to
>  	# filter out intentional WARNINGs or Oopses
> -	filter=${1:-cat}
> +	local filter=${1:-cat}
>  
>  	_dmesg_since_test_start | $filter >$seqres.dmesg
>  	egrep -q -e "kernel BUG at" \
> @@ -3680,7 +3690,7 @@ get_page_size()
>  run_fsx()
>  {
>  	echo fsx $@
> -	args=`echo $@ | sed -e "s/ BSIZE / $bsize /g" -e "s/ PSIZE / $psize /g"`
> +	local args=`echo $@ | sed -e "s/ BSIZE / $bsize /g" -e "s/ PSIZE / $psize /g"`
>  	set -- $here/ltp/fsx $args $FSX_AVOID $TEST_DIR/junk
>  	echo "$@" >>$seqres.full
>  	rm -f $TEST_DIR/junk
> -- 
> 2.17.0.484.g0c8726318c-goog
> 
> --
> To unsubscribe from this list: send the line "unsubscribe fstests" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html