linux-ext4 - Re: [PATCH v2 2/2] check: Replace exit with

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] [thread-next>] [day] [month] [year] [list]
Message-ID: <aBFn6YL3vHpb09yB@dread.disaster.area>
Date: Wed, 30 Apr 2025 09:59:37 +1000
From: Dave Chinner <david@...morbit.com>
To: "Nirjhar Roy (IBM)" <nirjhar.roy.lists@...il.com>
Cc: fstests@...r.kernel.org, linux-ext4@...r.kernel.org,
	linux-xfs@...r.kernel.org, ritesh.list@...il.com,
	ojaswin@...ux.ibm.com, djwong@...nel.org, zlang@...nel.org
Subject: Re: [PATCH v2 2/2] check: Replace exit with _exit in check

On Tue, Apr 29, 2025 at 06:52:54AM +0000, Nirjhar Roy (IBM) wrote:
> Some of the "status=<val>;exit" and "exit <val>" were not
> replace with _exit <val>. Doing it now.
> 
> Signed-off-by: Nirjhar Roy (IBM) <nirjhar.roy.lists@...il.com>
> ---
>  check | 44 ++++++++++++++++++--------------------------
>  1 file changed, 18 insertions(+), 26 deletions(-)
> 
> diff --git a/check b/check
> index 9451c350..99d38492 100755
> --- a/check
> +++ b/check
> @@ -47,6 +47,7 @@ export DIFF_LENGTH=${DIFF_LENGTH:=10}
>  # by default don't output timestamps
>  timestamp=${TIMESTAMP:=false}
>  
> +. common/exit
>  rm -f $tmp.list $tmp.tmp $tmp.grep $here/$iam.out $tmp.report.* $tmp.arglist
>  
>  SRC_GROUPS="generic"
> @@ -121,7 +122,7 @@ examples:
>   check -X .exclude -g auto
>   check -E ~/.xfstests.exclude
>  '
> -	    exit 1
> +	    _exit 1
>  }
>  
>  get_sub_group_list()
> @@ -231,7 +232,7 @@ _prepare_test_list()
>  			list=$(get_group_list $group)
>  			if [ -z "$list" ]; then
>  				echo "Group \"$group\" is empty or not defined?"
> -				exit 1
> +				_exit 1
>  			fi
>  
>  			for t in $list; do

This is now:

	_fatal "Group \"$group\" is empty or not defined?"

> @@ -316,14 +317,14 @@ while [ $# -gt 0 ]; do
>  	-r)
>  		if $exact_order; then
>  			echo "Cannot specify -r and --exact-order."
> -			exit 1
> +			_exit 1
>  		fi
>  		randomize=true
>  		;;
>  	--exact-order)
>  		if $randomize; then
>  			echo "Cannnot specify --exact-order and -r."
> -			exit 1
> +			_exit 1
>  		fi
>  		exact_order=true
>  		;;

Same.

> @@ -361,7 +362,7 @@ done
>  # after processing args, overlay needs FSTYP set before sourcing common/config
>  if ! . ./common/rc; then
>  	echo "check: failed to source common/rc"
> -	exit 1
> +	_exit 1
>  fi
>  
>  init_rc

Same.

> @@ -373,8 +374,7 @@ if [ -n "$SOAK_DURATION" ]; then
>  		sed -e 's/^\([.0-9]*\)\([a-z]\)*/\1 \2/g' | \
>  		$AWK_PROG -f $here/src/soak_duration.awk)"
>  	if [ $? -ne 0 ]; then
> -		status=1
> -		exit 1
> +		_exit 1
>  	fi
>  fi
>  
> @@ -385,8 +385,7 @@ if [ -n "$FUZZ_REWRITE_DURATION" ]; then
>  		sed -e 's/^\([.0-9]*\)\([a-z]\)*/\1 \2/g' | \
>  		$AWK_PROG -f $here/src/soak_duration.awk)"
>  	if [ $? -ne 0 ]; then
> -		status=1
> -		exit 1
> +		_exit 1
>  	fi
>  fi
>  
> @@ -404,8 +403,7 @@ if $have_test_arg; then
>  	while [ $# -gt 0 ]; do
>  		case "$1" in
>  		-*)	echo "Arguments before tests, please!"
> -			status=1
> -			exit $status
> +			_exit 1

_fatal

>  			;;
>  		*)	# Expand test pattern (e.g. xfs/???, *fs/001)
>  			list=$(cd $SRC_DIR; echo $1)
> @@ -438,7 +436,7 @@ fi
>  if [ `id -u` -ne 0 ]
>  then
>      echo "check: QA must be run as root"
> -    exit 1
> +    _exit 1
>  fi

Same

>  
>  _wipe_counters()
> @@ -721,9 +719,9 @@ _prepare_test_list
>  fstests_start_time="$(date +"%F %T")"
>  
>  if $OPTIONS_HAVE_SECTIONS; then
> -	trap "_summary; exit \$status" 0 1 2 3 15
> +	trap "_summary; _exit" 0 1 2 3 15
>  else
> -	trap "_wrapup; exit \$status" 0 1 2 3 15
> +	trap "_wrapup; _exit" 0 1 2 3 15
>  fi

Please add a comment explaining that _exit will capture $status
that has been previously set as the exit value.

Realistically, though, I think 'exit $status' is much better here
because it clearly documents that we are capturing $status as the
exit value from the trap rather than having to add a comment to make
it clear that $status is the exit value of the trap...

>  function run_section()
> @@ -767,8 +765,7 @@ function run_section()
>  	mkdir -p $RESULT_BASE
>  	if [ ! -d $RESULT_BASE ]; then
>  		echo "failed to create results directory $RESULT_BASE"
> -		status=1
> -		exit
> +		_exit 1
>  	fi

_fatal

>  	if $OPTIONS_HAVE_SECTIONS; then
> @@ -784,8 +781,7 @@ function run_section()
>  			echo "our local _test_mkfs routine ..."
>  			cat $tmp.err
>  			echo "check: failed to mkfs \$TEST_DEV using specified options"
> -			status=1
> -			exit
> +			_exit 1
>  		fi
>  		# Previous FSTYP derived from TEST_DEV could be changed, source
>  		# common/rc again with correct FSTYP to get FSTYP specific configs,
> @@ -829,8 +825,7 @@ function run_section()
>  	      echo "our local _scratch_mkfs routine ..."
>  	      cat $tmp.err
>  	      echo "check: failed to mkfs \$SCRATCH_DEV using specified options"
> -	      status=1
> -	      exit
> +	      _exit 1
>  	  fi
>  
>  	  # call the overridden mount - make sure the FS mounts with
> @@ -840,8 +835,7 @@ function run_section()
>  	      echo "our local mount routine ..."
>  	      cat $tmp.err
>  	      echo "check: failed to mount \$SCRATCH_DEV using specified options"
> -	      status=1
> -	      exit
> +	      _exit 1
>  	  else
>  	      _scratch_unmount
>  	  fi

Same for all these.

-Dave.
-- 
Dave Chinner
david@...morbit.com