Re: [PATCH v2 2/2] check: Replace exit with _exit in check

["Nirjhar Roy (IBM)" <nirjhar.roy.lists@gmail.com>]

On 4/30/25 05:29, Dave Chinner wrote:
> 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@gmail.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?"
Noted.
>
>> @@ -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.
Noted.
>
>> @@ -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.

Noted.

>
>> @@ -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
Noted.
>
>>   			;;
>>   		*)	# 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
Noted.
>
>>   
>>   _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...

Yeah, I will use "exit \$status" and mention in comments as to why we 
aren't using _exit(), although we can.

>
>>   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
Noted.
>
>>   	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.
Noted.
>
> -Dave.

-- 
Nirjhar Roy
Linux Kernel Developer
IBM, Bangalore