[PATCH v4 0/6] Minor cleanups in common/

public inbox for linux-xfs@vger.kernel.org
 help / color / mirror / Atom feed

* [PATCH v4 0/6] Minor cleanups in common/
@ 2025-04-09  7:00 Nirjhar Roy (IBM)
  2025-04-09  7:00 ` [PATCH v4 1/6] generic/749: Remove redundant sourcing of common/rc Nirjhar Roy (IBM)
                   ` (5 more replies)
  0 siblings, 6 replies; 15+ messages in thread
From: Nirjhar Roy (IBM) @ 2025-04-09  7:00 UTC (permalink / raw)
  To: fstests
  Cc: linux-ext4, linux-xfs, ritesh.list, ojaswin, djwong, zlang, david,
	nirjhar.roy.lists

This patch series removes some unnecessary sourcing of common/rc
and decouples the call to init_rc() from the sourcing of common/rc.
This is proposed in [1] and [2]. It also removes direct usage of exit command
with a _exit wrapper. The individual patches have the details.

[v3] --> v4
 1. Added R.Bs from Zorro and Ritesh in patch 2 of [v3]
 2. Updated the definition of _exit() - explicitly set the value "status" only if it is passed.
 3. Updated the description comment for _exit().

[1] https://lore.kernel.org/all/20250206155251.GA21787@frogsfrogsfrogs/
[2] https://lore.kernel.org/all/20250210142322.tptpphdntglsz4eq@dell-per750-06-vm-08.rhts.eng.pek2.redhat.com/
[3] https://lore.kernel.org/all/Z-xcne3f5Klvuxcq@dread.disaster.area/

[v1] https://lore.kernel.org/all/cover.1741248214.git.nirjhar.roy.lists@gmail.com/
[v2] https://lore.kernel.org/all/cover.1743487913.git.nirjhar.roy.lists@gmail.com/
[v3] https://lore.kernel.org/all/cover.1744090313.git.nirjhar.roy.lists@gmail.com/

Nirjhar Roy (IBM) (6):
  generic/749: Remove redundant sourcing of common/rc
  generic/367: Remove redundant sourcing of common/config
  check: Remove redundant _test_mount in check
  check,common{rc,preamble}: Decouple init_rc() call from sourcing
    common/rc
  common/config: Introduce _exit wrapper around exit command
  common: exit --> _exit

 check             |  10 ++---
 common/btrfs      |   6 +--
 common/ceph       |   2 +-
 common/config     |  16 +++++--
 common/dump       |  11 +++--
 common/ext4       |   2 +-
 common/populate   |   2 +-
 common/preamble   |   3 +-
 common/punch      |  13 +++---
 common/rc         | 107 ++++++++++++++++++++++------------------------
 common/repair     |   4 +-
 common/xfs        |   8 ++--
 tests/generic/367 |   1 -
 tests/generic/749 |   1 -
 14 files changed, 92 insertions(+), 94 deletions(-)

--
2.34.1


^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v4 1/6] generic/749: Remove redundant sourcing of common/rc
  2025-04-09  7:00 [PATCH v4 0/6] Minor cleanups in common/ Nirjhar Roy (IBM)
@ 2025-04-09  7:00 ` Nirjhar Roy (IBM)
  2025-04-09  7:00 ` [PATCH v4 2/6] generic/367: Remove redundant sourcing of common/config Nirjhar Roy (IBM)
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 15+ messages in thread
From: Nirjhar Roy (IBM) @ 2025-04-09  7:00 UTC (permalink / raw)
  To: fstests
  Cc: linux-ext4, linux-xfs, ritesh.list, ojaswin, djwong, zlang, david,
	nirjhar.roy.lists

common/rc is already sourced before the test starts running
in _begin_fstest() preamble.

Signed-off-by: Nirjhar Roy (IBM) <nirjhar.roy.lists@gmail.com>
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
Reviewed-by: Dave Chinner <dchinner@redhat.com>
Reviewed-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>
---
 tests/generic/749 | 1 -
 1 file changed, 1 deletion(-)

diff --git a/tests/generic/749 b/tests/generic/749
index fc747738..451f283e 100755
--- a/tests/generic/749
+++ b/tests/generic/749
@@ -15,7 +15,6 @@
 # boundary and ensures we get a SIGBUS if we write to data beyond the system
 # page size even if the block size is greater than the system page size.
 . ./common/preamble
-. ./common/rc
 _begin_fstest auto quick prealloc
 
 # Import common functions.
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v4 2/6] generic/367: Remove redundant sourcing of common/config
  2025-04-09  7:00 [PATCH v4 0/6] Minor cleanups in common/ Nirjhar Roy (IBM)
  2025-04-09  7:00 ` [PATCH v4 1/6] generic/749: Remove redundant sourcing of common/rc Nirjhar Roy (IBM)
@ 2025-04-09  7:00 ` Nirjhar Roy (IBM)
  2025-04-09 15:21   ` Darrick J. Wong
  2025-04-09  7:00 ` [PATCH v4 3/6] check: Remove redundant _test_mount in check Nirjhar Roy (IBM)
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 15+ messages in thread
From: Nirjhar Roy (IBM) @ 2025-04-09  7:00 UTC (permalink / raw)
  To: fstests
  Cc: linux-ext4, linux-xfs, ritesh.list, ojaswin, djwong, zlang, david,
	nirjhar.roy.lists

common/config will be source by _begin_fstest

Signed-off-by: Nirjhar Roy (IBM) <nirjhar.roy.lists@gmail.com>
Reviewed-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>
Reviewed-by: Zorro Lang <zlang@redhat.com>
---
 tests/generic/367 | 1 -
 1 file changed, 1 deletion(-)

diff --git a/tests/generic/367 b/tests/generic/367
index ed371a02..567db557 100755
--- a/tests/generic/367
+++ b/tests/generic/367
@@ -11,7 +11,6 @@
 # check if the extsize value and the xflag bit actually got reflected after
 # setting/re-setting the extsize value.
 
-. ./common/config
 . ./common/filter
 . ./common/preamble
 
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v4 3/6] check: Remove redundant _test_mount in check
  2025-04-09  7:00 [PATCH v4 0/6] Minor cleanups in common/ Nirjhar Roy (IBM)
  2025-04-09  7:00 ` [PATCH v4 1/6] generic/749: Remove redundant sourcing of common/rc Nirjhar Roy (IBM)
  2025-04-09  7:00 ` [PATCH v4 2/6] generic/367: Remove redundant sourcing of common/config Nirjhar Roy (IBM)
@ 2025-04-09  7:00 ` Nirjhar Roy (IBM)
  2025-04-09 15:21   ` Darrick J. Wong
  2025-04-09  7:00 ` [PATCH v4 4/6] check,common{rc,preamble}: Decouple init_rc() call from sourcing common/rc Nirjhar Roy (IBM)
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 15+ messages in thread
From: Nirjhar Roy (IBM) @ 2025-04-09  7:00 UTC (permalink / raw)
  To: fstests
  Cc: linux-ext4, linux-xfs, ritesh.list, ojaswin, djwong, zlang, david,
	nirjhar.roy.lists

init_rc already does a _test_mount. Hence removing the additional
_test_mount call when OLD_TEST_FS_MOUNT_OPTS != TEST_FS_MOUNT_OPTS.

Signed-off-by: Nirjhar Roy (IBM) <nirjhar.roy.lists@gmail.com>
Reviewed-by: Dave Chinner <dchinner@redhat.com>
Reviewed-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>
---
 check | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/check b/check
index 32890470..3d621210 100755
--- a/check
+++ b/check
@@ -791,13 +791,9 @@ function run_section()
 		. common/rc
 		_prepare_test_list
 	elif [ "$OLD_TEST_FS_MOUNT_OPTS" != "$TEST_FS_MOUNT_OPTS" ]; then
+		# Unmount TEST_DEV to apply the updated mount options.
+		# It will be mounted again by init_rc(), called shortly after.
 		_test_unmount 2> /dev/null
-		if ! _test_mount
-		then
-			echo "check: failed to mount $TEST_DEV on $TEST_DIR"
-			status=1
-			exit
-		fi
 	fi
 
 	init_rc
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v4 4/6] check,common{rc,preamble}: Decouple init_rc() call from sourcing common/rc
  2025-04-09  7:00 [PATCH v4 0/6] Minor cleanups in common/ Nirjhar Roy (IBM)
                   ` (2 preceding siblings ...)
  2025-04-09  7:00 ` [PATCH v4 3/6] check: Remove redundant _test_mount in check Nirjhar Roy (IBM)
@ 2025-04-09  7:00 ` Nirjhar Roy (IBM)
  2025-04-09 15:22   ` Darrick J. Wong
  2025-04-09  7:00 ` [PATCH v4 5/6] common/config: Introduce _exit wrapper around exit command Nirjhar Roy (IBM)
  2025-04-09  7:00 ` [PATCH v4 6/6] common: exit --> _exit Nirjhar Roy (IBM)
  5 siblings, 1 reply; 15+ messages in thread
From: Nirjhar Roy (IBM) @ 2025-04-09  7:00 UTC (permalink / raw)
  To: fstests
  Cc: linux-ext4, linux-xfs, ritesh.list, ojaswin, djwong, zlang, david,
	nirjhar.roy.lists

Silently executing scripts during sourcing common/rc isn't good practice
and also causes unnecessary script execution. Decouple init_rc() call
and call init_rc() explicitly where required.

Signed-off-by: Nirjhar Roy (IBM) <nirjhar.roy.lists@gmail.com>
Reviewed-by: Dave Chinner <dchinner@redhat.com>
Reviewed-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>
---
 check           | 2 ++
 common/preamble | 1 +
 common/rc       | 2 --
 3 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/check b/check
index 3d621210..9451c350 100755
--- a/check
+++ b/check
@@ -364,6 +364,8 @@ if ! . ./common/rc; then
 	exit 1
 fi
 
+init_rc
+
 # If the test config specified a soak test duration, see if there are any
 # unit suffixes that need converting to an integer seconds count.
 if [ -n "$SOAK_DURATION" ]; then
diff --git a/common/preamble b/common/preamble
index 0c9ee2e0..c92e55bb 100644
--- a/common/preamble
+++ b/common/preamble
@@ -50,6 +50,7 @@ _begin_fstest()
 	_register_cleanup _cleanup
 
 	. ./common/rc
+	init_rc
 
 	# remove previous $seqres.full before test
 	rm -f $seqres.full $seqres.hints
diff --git a/common/rc b/common/rc
index 16d627e1..038c22f6 100644
--- a/common/rc
+++ b/common/rc
@@ -5817,8 +5817,6 @@ _require_program() {
 	_have_program "$1" || _notrun "$tag required"
 }
 
-init_rc
-
 ################################################################################
 # make sure this script returns success
 /bin/true
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v4 5/6] common/config: Introduce _exit wrapper around exit command
  2025-04-09  7:00 [PATCH v4 0/6] Minor cleanups in common/ Nirjhar Roy (IBM)
                   ` (3 preceding siblings ...)
  2025-04-09  7:00 ` [PATCH v4 4/6] check,common{rc,preamble}: Decouple init_rc() call from sourcing common/rc Nirjhar Roy (IBM)
@ 2025-04-09  7:00 ` Nirjhar Roy (IBM)
  2025-04-09 15:23   ` Darrick J. Wong
  2025-04-09  7:00 ` [PATCH v4 6/6] common: exit --> _exit Nirjhar Roy (IBM)
  5 siblings, 1 reply; 15+ messages in thread
From: Nirjhar Roy (IBM) @ 2025-04-09  7:00 UTC (permalink / raw)
  To: fstests
  Cc: linux-ext4, linux-xfs, ritesh.list, ojaswin, djwong, zlang, david,
	nirjhar.roy.lists

We should always set the value of status correctly when we are exiting.
Else, "$?" might not give us the correct value.
If we see the following trap
handler registration in the check script:

if $OPTIONS_HAVE_SECTIONS; then
     trap "_kill_seq; _summary; exit \$status" 0 1 2 3 15
else
     trap "_kill_seq; _wrapup; exit \$status" 0 1 2 3 15
fi

So, "exit 1" will exit the check script without setting the correct
return value. I ran with the following local.config file:

[xfs_4k_valid]
FSTYP=xfs
TEST_DEV=/dev/loop0
TEST_DIR=/mnt1/test
SCRATCH_DEV=/dev/loop1
SCRATCH_MNT=/mnt1/scratch

[xfs_4k_invalid]
FSTYP=xfs
TEST_DEV=/dev/loop0
TEST_DIR=/mnt1/invalid_dir
SCRATCH_DEV=/dev/loop1
SCRATCH_MNT=/mnt1/scratch

This caused the init_rc() to catch the case of invalid _test_mount
options. Although the check script correctly failed during the execution
of the "xfs_4k_invalid" section, the return value was 0, i.e "echo $?"
returned 0. This is because init_rc exits with "exit 1" without
correctly setting the value of "status". IMO, the correct behavior
should have been that "$?" should have been non-zero.

The next patch will replace exit with _exit.

Signed-off-by: Nirjhar Roy (IBM) <nirjhar.roy.lists@gmail.com>
Reviewed-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>
Reviewed-by: Dave Chinner <dchinner@redhat.com>
---
 common/config | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/common/config b/common/config
index 79bec87f..7dd78dbe 100644
--- a/common/config
+++ b/common/config
@@ -96,6 +96,15 @@ export LOCAL_CONFIGURE_OPTIONS=${LOCAL_CONFIGURE_OPTIONS:=--enable-readline=yes}
 
 export RECREATE_TEST_DEV=${RECREATE_TEST_DEV:=false}
 
+# This functions sets the exit code to status and then exits. Don't use
+# exit directly, as it might not set the value of "$status" correctly, which is
+# used as an exit code in the trap handler routine set up by the check script.
+_exit()
+{
+	test -n "$1" && status="$1"
+	exit "$status"
+}
+
 # Handle mkfs.$fstyp which does (or does not) require -f to overwrite
 set_mkfs_prog_path_with_opts()
 {
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v4 6/6] common: exit --> _exit
  2025-04-09  7:00 [PATCH v4 0/6] Minor cleanups in common/ Nirjhar Roy (IBM)
                   ` (4 preceding siblings ...)
  2025-04-09  7:00 ` [PATCH v4 5/6] common/config: Introduce _exit wrapper around exit command Nirjhar Roy (IBM)
@ 2025-04-09  7:00 ` Nirjhar Roy (IBM)
  2025-04-09 15:24   ` Darrick J. Wong
  5 siblings, 1 reply; 15+ messages in thread
From: Nirjhar Roy (IBM) @ 2025-04-09  7:00 UTC (permalink / raw)
  To: fstests
  Cc: linux-ext4, linux-xfs, ritesh.list, ojaswin, djwong, zlang, david,
	nirjhar.roy.lists

Replace exit <return-val> with _exit <return-val> which
is introduced in the previous patch.

Signed-off-by: Nirjhar Roy (IBM) <nirjhar.roy.lists@gmail.com>
Reviewed-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>
Reviewed-by: Dave Chinner <dchinner@redhat.com>
---
 common/btrfs    |   6 +--
 common/ceph     |   2 +-
 common/config   |   7 ++--
 common/dump     |  11 +++--
 common/ext4     |   2 +-
 common/populate |   2 +-
 common/preamble |   2 +-
 common/punch    |  13 +++---
 common/rc       | 105 +++++++++++++++++++++++-------------------------
 common/repair   |   4 +-
 common/xfs      |   8 ++--
 11 files changed, 78 insertions(+), 84 deletions(-)

diff --git a/common/btrfs b/common/btrfs
index a3b9c12f..3725632c 100644
--- a/common/btrfs
+++ b/common/btrfs
@@ -80,7 +80,7 @@ _require_btrfs_mkfs_feature()
 {
 	if [ -z $1 ]; then
 		echo "Missing feature name argument for _require_btrfs_mkfs_feature"
-		exit 1
+		_exit 1
 	fi
 	feat=$1
 	$MKFS_BTRFS_PROG -O list-all 2>&1 | \
@@ -104,7 +104,7 @@ _require_btrfs_fs_feature()
 {
 	if [ -z $1 ]; then
 		echo "Missing feature name argument for _require_btrfs_fs_feature"
-		exit 1
+		_exit 1
 	fi
 	feat=$1
 	modprobe btrfs > /dev/null 2>&1
@@ -214,7 +214,7 @@ _check_btrfs_filesystem()
 	if [ $ok -eq 0 ]; then
 		status=1
 		if [ "$iam" != "check" ]; then
-			exit 1
+			_exit 1
 		fi
 		return 1
 	fi
diff --git a/common/ceph b/common/ceph
index d6f24df1..df7a6814 100644
--- a/common/ceph
+++ b/common/ceph
@@ -14,7 +14,7 @@ _ceph_create_file_layout()
 
 	if [ -e $fname ]; then
 		echo "File $fname already exists."
-		exit 1
+		_exit 1
 	fi
 	touch $fname
 	$SETFATTR_PROG -n ceph.file.layout \
diff --git a/common/config b/common/config
index 7dd78dbe..eada3971 100644
--- a/common/config
+++ b/common/config
@@ -124,8 +124,7 @@ set_mkfs_prog_path_with_opts()
 _fatal()
 {
     echo "$*"
-    status=1
-    exit 1
+    _exit 1
 }
 
 export MKFS_PROG="$(type -P mkfs)"
@@ -869,7 +868,7 @@ get_next_config() {
 		echo "Warning: need to define parameters for host $HOST"
 		echo "       or set variables:"
 		echo "       $MC"
-		exit 1
+		_exit 1
 	fi
 
 	_check_device TEST_DEV required $TEST_DEV
@@ -880,7 +879,7 @@ get_next_config() {
 	if [ ! -z "$SCRATCH_DEV_POOL" ]; then
 		if [ ! -z "$SCRATCH_DEV" ]; then
 			echo "common/config: Error: \$SCRATCH_DEV ($SCRATCH_DEV) should be unset when \$SCRATCH_DEV_POOL ($SCRATCH_DEV_POOL) is set"
-			exit 1
+			_exit 1
 		fi
 		SCRATCH_DEV=`echo $SCRATCH_DEV_POOL | awk '{print $1}'`
 		export SCRATCH_DEV
diff --git a/common/dump b/common/dump
index 6dcd6250..09859006 100644
--- a/common/dump
+++ b/common/dump
@@ -280,8 +280,7 @@ _create_dumpdir_stress_num()
     rm -rf $dump_dir
     if ! mkdir $dump_dir; then
         echo "    failed to mkdir $dump_dir"
-        status=1
-        exit
+        _exit 1
     fi
 
     # Remove fsstress commands that aren't supported on all xfs configs so that
@@ -480,7 +479,7 @@ _do_create_dumpdir_fill()
 		else
 		    $verbose && echo
 		    echo "Error: cannot mkdir \"$dir\""
-		    exit 1
+		    _exit 1
 		fi
 	    fi
 	else
@@ -496,7 +495,7 @@ _do_create_dumpdir_fill()
 		    else
 			$verbose && echo
 			echo "Error: cannot mkdir \"$dir\""
-			exit 1
+			_exit 1
 		    fi
 		fi
 	    fi
@@ -507,7 +506,7 @@ _do_create_dumpdir_fill()
 	    else
 		$verbose && echo
 		echo "Error: cannot create \"$file\""
-		exit 1
+		_exit 1
 	    fi
 	fi
 	if [ -n "$owner" -a -n "$group" ]; then
@@ -649,7 +648,7 @@ _do_create_dump_symlinks()
 		else
 		    $verbose && echo
 		    echo "Error: cannot mkdir \"$dir\""
-		    exit 1
+		    _exit 1
 		fi
 	    fi
 	fi
diff --git a/common/ext4 b/common/ext4
index e1b336d3..f88fa532 100644
--- a/common/ext4
+++ b/common/ext4
@@ -182,7 +182,7 @@ _require_scratch_ext4_feature()
 {
     if [ -z "$1" ]; then
         echo "Usage: _require_scratch_ext4_feature feature"
-        exit 1
+        _exit 1
     fi
     $MKFS_EXT4_PROG -F $MKFS_OPTIONS -O "$1" \
 		    $SCRATCH_DEV 512m >/dev/null 2>&1 \
diff --git a/common/populate b/common/populate
index 7352f598..50dc75d3 100644
--- a/common/populate
+++ b/common/populate
@@ -1003,7 +1003,7 @@ _fill_fs()
 
 	if [ $# -ne 4 ]; then
 		echo "Usage: _fill_fs filesize dir blocksize switch_user"
-		exit 1
+		_exit 1
 	fi
 
 	if [ $switch_user -eq 0 ]; then
diff --git a/common/preamble b/common/preamble
index c92e55bb..ba029a34 100644
--- a/common/preamble
+++ b/common/preamble
@@ -35,7 +35,7 @@ _begin_fstest()
 {
 	if [ -n "$seq" ]; then
 		echo "_begin_fstest can only be called once!"
-		exit 1
+		_exit 1
 	fi
 
 	seq=`basename $0`
diff --git a/common/punch b/common/punch
index 43ccab69..64d665d8 100644
--- a/common/punch
+++ b/common/punch
@@ -172,16 +172,16 @@ _filter_fiemap_flags()
 	$AWK_PROG -e "$awk_script" | _coalesce_extents
 }
 
-# Filters fiemap output to only print the 
+# Filters fiemap output to only print the
 # file offset column and whether or not
 # it is an extent or a hole
 _filter_hole_fiemap()
 {
 	$AWK_PROG '
 		$3 ~ /hole/ {
-			print $1, $2, $3; 
+			print $1, $2, $3;
 			next;
-		}   
+		}
 		$5 ~ /0x[[:xdigit:]]+/ {
 			print $1, $2, "extent";
 		}' |
@@ -224,8 +224,7 @@ _filter_bmap()
 
 die_now()
 {
-	status=1
-	exit
+	_exit 1
 }
 
 # test the different corner cases for zeroing a range:
@@ -276,7 +275,7 @@ _test_generic_punch()
 		u)	unwritten_tests=
 		;;
 		?)	echo Invalid flag
-		exit 1
+		_exit 1
 		;;
 		esac
 	done
@@ -552,7 +551,7 @@ _test_block_boundaries()
 		d)	sync_cmd=
 		;;
 		?)	echo Invalid flag
-		exit 1
+		_exit 1
 		;;
 		esac
 	done
diff --git a/common/rc b/common/rc
index 038c22f6..3b21eb27 100644
--- a/common/rc
+++ b/common/rc
@@ -909,8 +909,7 @@ _mkfs_dev()
 	# output stored mkfs output
 	cat $tmp.mkfserr >&2
 	cat $tmp.mkfsstd
-	status=1
-	exit 1
+	_exit 1
     fi
     rm -f $tmp.mkfserr $tmp.mkfsstd
 }
@@ -1575,7 +1574,7 @@ _get_pids_by_name()
     if [ $# -ne 1 ]
     then
 	echo "Usage: _get_pids_by_name process-name" 1>&2
-	exit 1
+	_exit 1
     fi
 
     # Algorithm ... all ps(1) variants have a time of the form MM:SS or
@@ -1609,7 +1608,7 @@ _df_device()
     if [ $# -ne 1 ]
     then
 	echo "Usage: _df_device device" 1>&2
-	exit 1
+	_exit 1
     fi
 
     # Note that we use "==" here so awk doesn't try to interpret an NFS over
@@ -1641,7 +1640,7 @@ _df_dir()
     if [ $# -ne 1 ]
     then
 	echo "Usage: _df_dir device" 1>&2
-	exit 1
+	_exit 1
     fi
 
     $DF_PROG $1 2>/dev/null | $AWK_PROG -v what=$1 '
@@ -1667,7 +1666,7 @@ _used()
     if [ $# -ne 1 ]
     then
 	echo "Usage: _used device" 1>&2
-	exit 1
+	_exit 1
     fi
 
     _df_device $1 | $AWK_PROG '{ sub("%", "") ; print $6 }'
@@ -1680,7 +1679,7 @@ _fs_type()
     if [ $# -ne 1 ]
     then
 	echo "Usage: _fs_type device" 1>&2
-	exit 1
+	_exit 1
     fi
 
     #
@@ -1705,7 +1704,7 @@ _fs_options()
     if [ $# -ne 1 ]
     then
 	echo "Usage: _fs_options device" 1>&2
-	exit 1
+	_exit 1
     fi
 
     $AWK_PROG -v dev=$1 '
@@ -1720,7 +1719,7 @@ _is_block_dev()
     if [ $# -ne 1 ]
     then
 	echo "Usage: _is_block_dev dev" 1>&2
-	exit 1
+	_exit 1
     fi
 
     local dev=$1
@@ -1739,7 +1738,7 @@ _is_char_dev()
 {
 	if [ $# -ne 1 ]; then
 		echo "Usage: _is_char_dev dev" 1>&2
-		exit 1
+		_exit 1
 	fi
 
 	local dev=$1
@@ -1772,7 +1771,7 @@ _do()
 	echo -n "$note... "
     else
 	echo "Usage: _do [note] cmd" 1>&2
-	status=1; exit
+	_exit 1
     fi
 
     (eval "echo '---' \"$cmd\"") >>$seqres.full
@@ -1793,7 +1792,7 @@ _do()
     then
 	[ $# -ne 2 ] && echo
 	eval "echo \"$cmd\" failed \(returned $ret\): see $seqres.full"
-	status=1; exit
+	_exit 1
     fi
 
     return $ret
@@ -1809,8 +1808,7 @@ _notrun()
     rm -f ${RESULT_DIR}/require_test*
     rm -f ${RESULT_DIR}/require_scratch*
 
-    status=0
-    exit
+    _exit 0
 }
 
 # just plain bail out
@@ -1819,8 +1817,7 @@ _fail()
 {
     echo "$*" | tee -a $seqres.full
     echo "(see $seqres.full for details)"
-    status=1
-    exit 1
+    _exit 1
 }
 
 #
@@ -2049,14 +2046,14 @@ _require_scratch_nocheck()
 
     _check_mounted_on SCRATCH_DEV $SCRATCH_DEV SCRATCH_MNT $SCRATCH_MNT
     local err=$?
-    [ $err -le 1 ] || exit 1
+    [ $err -le 1 ] || _exit 1
     if [ $err -eq 0 ]
     then
         # if it's mounted, unmount it
         if ! _scratch_unmount
         then
             echo "failed to unmount $SCRATCH_DEV"
-            exit 1
+            _exit 1
         fi
     fi
     rm -f ${RESULT_DIR}/require_scratch "$RESULT_DIR/.skip_orebuild" "$RESULT_DIR/.skip_rebuild"
@@ -2273,13 +2270,13 @@ _require_test()
 
     _check_mounted_on TEST_DEV $TEST_DEV TEST_DIR $TEST_DIR
     local err=$?
-    [ $err -le 1 ] || exit 1
+    [ $err -le 1 ] || _exit 1
     if [ $err -ne 0 ]
     then
 	if ! _test_mount
 	then
 		echo "!!! failed to mount $TEST_DEV on $TEST_DIR"
-		exit 1
+		_exit 1
 	fi
     fi
     touch ${RESULT_DIR}/require_test
@@ -2391,7 +2388,7 @@ _require_block_device()
 {
 	if [ -z "$1" ]; then
 		echo "Usage: _require_block_device <dev>" 1>&2
-		exit 1
+		_exit 1
 	fi
 	if [ "`_is_block_dev "$1"`" == "" ]; then
 		_notrun "require $1 to be valid block disk"
@@ -2404,7 +2401,7 @@ _require_local_device()
 {
 	if [ -z "$1" ]; then
 		echo "Usage: _require_local_device <dev>" 1>&2
-		exit 1
+		_exit 1
 	fi
 	if [ "`_is_block_dev "$1"`" != "" ]; then
 		return 0
@@ -2512,7 +2509,7 @@ _zone_type()
 	local target=$1
 	if [ -z $target ]; then
 		echo "Usage: _zone_type <device>"
-		exit 1
+		_exit 1
 	fi
 	local sdev=`_short_dev $target`
 
@@ -2528,7 +2525,7 @@ _require_zoned_device()
 	local target=$1
 	if [ -z $target ]; then
 		echo "Usage: _require_zoned_device <device>"
-		exit 1
+		_exit 1
 	fi
 
 	local type=`_zone_type ${target}`
@@ -2668,7 +2665,7 @@ _run_aiodio()
     if [ -z "$1" ]
     then
         echo "usage: _run_aiodio command_name" 2>&1
-        status=1; exit 1
+        _exit 1
     fi
 
     _require_aiodio $1
@@ -2880,7 +2877,7 @@ _require_xfs_io_command()
 	if [ -z "$1" ]
 	then
 		echo "Usage: _require_xfs_io_command command [switch]" 1>&2
-		exit 1
+		_exit 1
 	fi
 	local command=$1
 	shift
@@ -3364,7 +3361,7 @@ _is_dev_mounted()
 
 	if [ $# -lt 1 ]; then
 		echo "Usage: _is_dev_mounted <device> [fstype]" 1>&2
-		exit 1
+		_exit 1
 	fi
 
 	findmnt -rncv -S $dev -t $fstype -o TARGET | head -1
@@ -3378,7 +3375,7 @@ _is_dir_mountpoint()
 
 	if [ $# -lt 1 ]; then
 		echo "Uasge: _is_dir_mountpoint <dir> [fstype]" 1>&2
-		exit 1
+		_exit 1
 	fi
 
 	findmnt -rncv -t $fstype -o TARGET $dir | head -1
@@ -3391,7 +3388,7 @@ _remount()
     if [ $# -ne 2 ]
     then
 	echo "Usage: _remount device ro/rw" 1>&2
-	exit 1
+	_exit 1
     fi
     local device=$1
     local mode=$2
@@ -3399,7 +3396,7 @@ _remount()
     if ! mount -o remount,$mode $device
     then
         echo "_remount: failed to remount filesystem on $device as $mode"
-        exit 1
+        _exit 1
     fi
 }
 
@@ -3417,7 +3414,7 @@ _umount_or_remount_ro()
     if [ $# -ne 1 ]
     then
 	echo "Usage: _umount_or_remount_ro <device>" 1>&2
-	exit 1
+	_exit 1
     fi
 
     local device=$1
@@ -3435,7 +3432,7 @@ _mount_or_remount_rw()
 {
 	if [ $# -ne 3 ]; then
 		echo "Usage: _mount_or_remount_rw <opts> <dev> <mnt>" 1>&2
-		exit 1
+		_exit 1
 	fi
 	local mount_opts=$1
 	local device=$2
@@ -3516,7 +3513,7 @@ _check_generic_filesystem()
     if [ $ok -eq 0 ]; then
 	status=1
 	if [ "$iam" != "check" ]; then
-		exit 1
+		_exit 1
 	fi
 	return 1
     fi
@@ -3582,7 +3579,7 @@ _check_udf_filesystem()
     if [ $# -ne 1 -a $# -ne 2 ]
     then
 	echo "Usage: _check_udf_filesystem device [last_block]" 1>&2
-	exit 1
+	_exit 1
     fi
 
     if [ ! -x $here/src/udf_test ]
@@ -3776,7 +3773,7 @@ _get_os_name()
 		echo 'linux'
 	else
 		echo Unknown operating system: `uname`
-		exit
+		_exit 1
 	fi
 }
 
@@ -3837,7 +3834,7 @@ _link_out_file()
 _die()
 {
         echo $@
-        exit 1
+        _exit 1
 }
 
 # convert urandom incompressible data to compressible text data
@@ -3994,7 +3991,7 @@ _require_scratch_dev_pool()
 		if _mount | grep -q $i; then
 			if ! _unmount $i; then
 		            echo "failed to unmount $i - aborting"
-		            exit 1
+		            _exit 1
 		        fi
 		fi
 		# To help better debug when something fails, we remove
@@ -4403,7 +4400,7 @@ _require_batched_discard()
 {
 	if [ $# -ne 1 ]; then
 		echo "Usage: _require_batched_discard mnt_point" 1>&2
-		exit 1
+		_exit 1
 	fi
 	_require_fstrim
 
@@ -4630,7 +4627,7 @@ _require_chattr()
 {
 	if [ -z "$1" ]; then
 		echo "Usage: _require_chattr <attr>"
-		exit 1
+		_exit 1
 	fi
 	local attribute=$1
 
@@ -4649,7 +4646,7 @@ _get_total_inode()
 {
 	if [ -z "$1" ]; then
 		echo "Usage: _get_total_inode <mnt>"
-		exit 1
+		_exit 1
 	fi
 	local nr_inode;
 	nr_inode=`$DF_PROG -i $1 | tail -1 | awk '{print $3}'`
@@ -4660,7 +4657,7 @@ _get_used_inode()
 {
 	if [ -z "$1" ]; then
 		echo "Usage: _get_used_inode <mnt>"
-		exit 1
+		_exit 1
 	fi
 	local nr_inode;
 	nr_inode=`$DF_PROG -i $1 | tail -1 | awk '{print $4}'`
@@ -4671,7 +4668,7 @@ _get_used_inode_percent()
 {
 	if [ -z "$1" ]; then
 		echo "Usage: _get_used_inode_percent <mnt>"
-		exit 1
+		_exit 1
 	fi
 	local pct_inode;
 	pct_inode=`$DF_PROG -i $1 | tail -1 | awk '{ print $6 }' | \
@@ -4683,7 +4680,7 @@ _get_free_inode()
 {
 	if [ -z "$1" ]; then
 		echo "Usage: _get_free_inode <mnt>"
-		exit 1
+		_exit 1
 	fi
 	local nr_inode;
 	nr_inode=`$DF_PROG -i $1 | tail -1 | awk '{print $5}'`
@@ -4696,7 +4693,7 @@ _get_available_space()
 {
 	if [ -z "$1" ]; then
 		echo "Usage: _get_available_space <mnt>"
-		exit 1
+		_exit 1
 	fi
 	$DF_PROG -B 1 $1 | tail -n1 | awk '{ print $5 }'
 }
@@ -4707,7 +4704,7 @@ _get_total_space()
 {
 	if [ -z "$1" ]; then
 		echo "Usage: _get_total_space <mnt>"
-		exit 1
+		_exit 1
 	fi
 	$DF_PROG -B 1 $1 | tail -n1 | awk '{ print $3 }'
 }
@@ -4952,7 +4949,7 @@ init_rc()
 	if [ "$TEST_DEV" = ""  ]
 	then
 		echo "common/rc: Error: \$TEST_DEV is not set"
-		exit 1
+		_exit 1
 	fi
 
 	# if $TEST_DEV is not mounted, mount it now as XFS
@@ -4966,20 +4963,20 @@ init_rc()
 			if ! _test_mount
 			then
 				echo "common/rc: could not mount $TEST_DEV on $TEST_DIR"
-				exit 1
+				_exit 1
 			fi
 		fi
 	fi
 
 	# Sanity check that TEST partition is not mounted at another mount point
 	# or as another fs type
-	_check_mounted_on TEST_DEV $TEST_DEV TEST_DIR $TEST_DIR $FSTYP || exit 1
+	_check_mounted_on TEST_DEV $TEST_DEV TEST_DIR $TEST_DIR $FSTYP || _exit 1
 	if [ -n "$SCRATCH_DEV" ]; then
 		# Sanity check that SCRATCH partition is not mounted at another
 		# mount point, because it is about to be unmounted and formatted.
 		# Another fs type for scratch is fine (bye bye old fs type).
 		_check_mounted_on SCRATCH_DEV $SCRATCH_DEV SCRATCH_MNT $SCRATCH_MNT
-		[ $? -le 1 ] || exit 1
+		[ $? -le 1 ] || _exit 1
 	fi
 
 	# Figure out if we need to add -F ("foreign", deprecated) option to xfs_io
@@ -5029,7 +5026,7 @@ _get_file_block_size()
 {
 	if [ -z $1 ] || [ ! -d $1 ]; then
 		echo "Missing mount point argument for _get_file_block_size"
-		exit 1
+		_exit 1
 	fi
 
 	case "$FSTYP" in
@@ -5076,7 +5073,7 @@ _get_block_size()
 {
 	if [ -z $1 ] || [ ! -d $1 ]; then
 		echo "Missing mount point argument for _get_block_size"
-		exit 1
+		_exit 1
 	fi
 	stat -f -c %S $1
 }
@@ -5146,14 +5143,14 @@ _run_hugepage_fsx() {
 	fi
 	cat $tmp.hugepage_fsx
 	rm -f $tmp.hugepage_fsx
-	test $res -ne 0 && exit 1
+	test $res -ne 0 && _exit 1
 	return 0
 }
 
 # run fsx or exit the test
 run_fsx()
 {
-	_run_fsx "$@" || exit 1
+	_run_fsx "$@" || _exit 1
 }
 
 _require_statx()
@@ -5318,7 +5315,7 @@ _get_max_file_size()
 {
 	if [ -z $1 ] || [ ! -d $1 ]; then
 		echo "Missing mount point argument for _get_max_file_size"
-		exit 1
+		_exit 1
 	fi
 
 	local mnt=$1
diff --git a/common/repair b/common/repair
index a79f9b2b..fd206f8e 100644
--- a/common/repair
+++ b/common/repair
@@ -16,7 +16,7 @@ _zero_position()
 		}'`
 	if [ -z "$offset" -o -z "$length" ]; then
 		echo "cannot calculate offset ($offset) or length ($length)"
-		exit
+		_exit 1
 	fi
 	length=`expr $length / 512`
 	$here/src/devzero -v $value -b 1 -n $length -o $offset $SCRATCH_DEV \
@@ -113,7 +113,7 @@ _filter_dd()
 }
 
 # do some controlled corrupting & ensure repair recovers us
-# 
+#
 _check_repair()
 {
 	value=$1
diff --git a/common/xfs b/common/xfs
index 81d568d3..96c15f3c 100644
--- a/common/xfs
+++ b/common/xfs
@@ -553,7 +553,7 @@ _require_xfs_db_command()
 {
 	if [ $# -ne 1 ]; then
 		echo "Usage: _require_xfs_db_command command" 1>&2
-		exit 1
+		_exit 1
 	fi
 	command=$1
 
@@ -789,7 +789,7 @@ _check_xfs_filesystem()
 
 	if [ $# -ne 3 ]; then
 		echo "Usage: _check_xfs_filesystem device <logdev>|none <rtdev>|none" 1>&2
-		exit 1
+		_exit 1
 	fi
 
 	extra_mount_options=""
@@ -1014,7 +1014,7 @@ _check_xfs_filesystem()
 	if [ $ok -eq 0 ]; then
 		status=1
 		if [ "$iam" != "check" ]; then
-			exit 1
+			_exit 1
 		fi
 		return 1
 	fi
@@ -1379,7 +1379,7 @@ _require_xfs_spaceman_command()
 {
 	if [ -z "$1" ]; then
 		echo "Usage: _require_xfs_spaceman_command command [switch]" 1>&2
-		exit 1
+		_exit 1
 	fi
 	local command=$1
 	shift
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH v4 2/6] generic/367: Remove redundant sourcing of common/config
  2025-04-09  7:00 ` [PATCH v4 2/6] generic/367: Remove redundant sourcing of common/config Nirjhar Roy (IBM)
@ 2025-04-09 15:21   ` Darrick J. Wong
  0 siblings, 0 replies; 15+ messages in thread
From: Darrick J. Wong @ 2025-04-09 15:21 UTC (permalink / raw)
  To: Nirjhar Roy (IBM)
  Cc: fstests, linux-ext4, linux-xfs, ritesh.list, ojaswin, zlang,
	david

On Wed, Apr 09, 2025 at 07:00:48AM +0000, Nirjhar Roy (IBM) wrote:
> common/config will be source by _begin_fstest
> 
> Signed-off-by: Nirjhar Roy (IBM) <nirjhar.roy.lists@gmail.com>
> Reviewed-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>
> Reviewed-by: Zorro Lang <zlang@redhat.com>

Yep, confirmed.
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

> ---
>  tests/generic/367 | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/tests/generic/367 b/tests/generic/367
> index ed371a02..567db557 100755
> --- a/tests/generic/367
> +++ b/tests/generic/367
> @@ -11,7 +11,6 @@
>  # check if the extsize value and the xflag bit actually got reflected after
>  # setting/re-setting the extsize value.
>  
> -. ./common/config
>  . ./common/filter
>  . ./common/preamble
>  
> -- 
> 2.34.1
> 
> 

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v4 3/6] check: Remove redundant _test_mount in check
  2025-04-09  7:00 ` [PATCH v4 3/6] check: Remove redundant _test_mount in check Nirjhar Roy (IBM)
@ 2025-04-09 15:21   ` Darrick J. Wong
  0 siblings, 0 replies; 15+ messages in thread
From: Darrick J. Wong @ 2025-04-09 15:21 UTC (permalink / raw)
  To: Nirjhar Roy (IBM)
  Cc: fstests, linux-ext4, linux-xfs, ritesh.list, ojaswin, zlang,
	david

On Wed, Apr 09, 2025 at 07:00:49AM +0000, Nirjhar Roy (IBM) wrote:
> init_rc already does a _test_mount. Hence removing the additional
> _test_mount call when OLD_TEST_FS_MOUNT_OPTS != TEST_FS_MOUNT_OPTS.
> 
> Signed-off-by: Nirjhar Roy (IBM) <nirjhar.roy.lists@gmail.com>
> Reviewed-by: Dave Chinner <dchinner@redhat.com>
> Reviewed-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>
> ---
>  check | 8 ++------
>  1 file changed, 2 insertions(+), 6 deletions(-)
> 
> diff --git a/check b/check
> index 32890470..3d621210 100755
> --- a/check
> +++ b/check
> @@ -791,13 +791,9 @@ function run_section()
>  		. common/rc
>  		_prepare_test_list
>  	elif [ "$OLD_TEST_FS_MOUNT_OPTS" != "$TEST_FS_MOUNT_OPTS" ]; then
> +		# Unmount TEST_DEV to apply the updated mount options.
> +		# It will be mounted again by init_rc(), called shortly after.

Thanks for adding this comment!  Future me will probably appreciate this
breadcrumb. :)
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

>  		_test_unmount 2> /dev/null
> -		if ! _test_mount
> -		then
> -			echo "check: failed to mount $TEST_DEV on $TEST_DIR"
> -			status=1
> -			exit
> -		fi
>  	fi
>  
>  	init_rc
> -- 
> 2.34.1
> 
> 

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v4 4/6] check,common{rc,preamble}: Decouple init_rc() call from sourcing common/rc
  2025-04-09  7:00 ` [PATCH v4 4/6] check,common{rc,preamble}: Decouple init_rc() call from sourcing common/rc Nirjhar Roy (IBM)
@ 2025-04-09 15:22   ` Darrick J. Wong
  0 siblings, 0 replies; 15+ messages in thread
From: Darrick J. Wong @ 2025-04-09 15:22 UTC (permalink / raw)
  To: Nirjhar Roy (IBM)
  Cc: fstests, linux-ext4, linux-xfs, ritesh.list, ojaswin, zlang,
	david

On Wed, Apr 09, 2025 at 07:00:50AM +0000, Nirjhar Roy (IBM) wrote:
> Silently executing scripts during sourcing common/rc isn't good practice
> and also causes unnecessary script execution. Decouple init_rc() call
> and call init_rc() explicitly where required.
> 
> Signed-off-by: Nirjhar Roy (IBM) <nirjhar.roy.lists@gmail.com>
> Reviewed-by: Dave Chinner <dchinner@redhat.com>
> Reviewed-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>

Sounds good to me
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

> ---
>  check           | 2 ++
>  common/preamble | 1 +
>  common/rc       | 2 --
>  3 files changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/check b/check
> index 3d621210..9451c350 100755
> --- a/check
> +++ b/check
> @@ -364,6 +364,8 @@ if ! . ./common/rc; then
>  	exit 1
>  fi
>  
> +init_rc
> +
>  # If the test config specified a soak test duration, see if there are any
>  # unit suffixes that need converting to an integer seconds count.
>  if [ -n "$SOAK_DURATION" ]; then
> diff --git a/common/preamble b/common/preamble
> index 0c9ee2e0..c92e55bb 100644
> --- a/common/preamble
> +++ b/common/preamble
> @@ -50,6 +50,7 @@ _begin_fstest()
>  	_register_cleanup _cleanup
>  
>  	. ./common/rc
> +	init_rc
>  
>  	# remove previous $seqres.full before test
>  	rm -f $seqres.full $seqres.hints
> diff --git a/common/rc b/common/rc
> index 16d627e1..038c22f6 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -5817,8 +5817,6 @@ _require_program() {
>  	_have_program "$1" || _notrun "$tag required"
>  }
>  
> -init_rc
> -
>  ################################################################################
>  # make sure this script returns success
>  /bin/true
> -- 
> 2.34.1
> 
> 

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v4 5/6] common/config: Introduce _exit wrapper around exit command
  2025-04-09  7:00 ` [PATCH v4 5/6] common/config: Introduce _exit wrapper around exit command Nirjhar Roy (IBM)
@ 2025-04-09 15:23   ` Darrick J. Wong
  0 siblings, 0 replies; 15+ messages in thread
From: Darrick J. Wong @ 2025-04-09 15:23 UTC (permalink / raw)
  To: Nirjhar Roy (IBM)
  Cc: fstests, linux-ext4, linux-xfs, ritesh.list, ojaswin, zlang,
	david

On Wed, Apr 09, 2025 at 07:00:51AM +0000, Nirjhar Roy (IBM) wrote:
> We should always set the value of status correctly when we are exiting.
> Else, "$?" might not give us the correct value.
> If we see the following trap
> handler registration in the check script:
> 
> if $OPTIONS_HAVE_SECTIONS; then
>      trap "_kill_seq; _summary; exit \$status" 0 1 2 3 15
> else
>      trap "_kill_seq; _wrapup; exit \$status" 0 1 2 3 15
> fi
> 
> So, "exit 1" will exit the check script without setting the correct
> return value. I ran with the following local.config file:
> 
> [xfs_4k_valid]
> FSTYP=xfs
> TEST_DEV=/dev/loop0
> TEST_DIR=/mnt1/test
> SCRATCH_DEV=/dev/loop1
> SCRATCH_MNT=/mnt1/scratch
> 
> [xfs_4k_invalid]
> FSTYP=xfs
> TEST_DEV=/dev/loop0
> TEST_DIR=/mnt1/invalid_dir
> SCRATCH_DEV=/dev/loop1
> SCRATCH_MNT=/mnt1/scratch
> 
> This caused the init_rc() to catch the case of invalid _test_mount
> options. Although the check script correctly failed during the execution
> of the "xfs_4k_invalid" section, the return value was 0, i.e "echo $?"
> returned 0. This is because init_rc exits with "exit 1" without
> correctly setting the value of "status". IMO, the correct behavior
> should have been that "$?" should have been non-zero.
> 
> The next patch will replace exit with _exit.
> 
> Signed-off-by: Nirjhar Roy (IBM) <nirjhar.roy.lists@gmail.com>
> Reviewed-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>
> Reviewed-by: Dave Chinner <dchinner@redhat.com>

Looks good now,
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

> ---
>  common/config | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/common/config b/common/config
> index 79bec87f..7dd78dbe 100644
> --- a/common/config
> +++ b/common/config
> @@ -96,6 +96,15 @@ export LOCAL_CONFIGURE_OPTIONS=${LOCAL_CONFIGURE_OPTIONS:=--enable-readline=yes}
>  
>  export RECREATE_TEST_DEV=${RECREATE_TEST_DEV:=false}
>  
> +# This functions sets the exit code to status and then exits. Don't use
> +# exit directly, as it might not set the value of "$status" correctly, which is
> +# used as an exit code in the trap handler routine set up by the check script.
> +_exit()
> +{
> +	test -n "$1" && status="$1"
> +	exit "$status"
> +}
> +
>  # Handle mkfs.$fstyp which does (or does not) require -f to overwrite
>  set_mkfs_prog_path_with_opts()
>  {
> -- 
> 2.34.1
> 
> 

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v4 6/6] common: exit --> _exit
  2025-04-09  7:00 ` [PATCH v4 6/6] common: exit --> _exit Nirjhar Roy (IBM)
@ 2025-04-09 15:24   ` Darrick J. Wong
  2025-04-09 16:29     ` Nirjhar Roy (IBM)
  2025-04-10 14:31     ` Zorro Lang
  0 siblings, 2 replies; 15+ messages in thread
From: Darrick J. Wong @ 2025-04-09 15:24 UTC (permalink / raw)
  To: Nirjhar Roy (IBM)
  Cc: fstests, linux-ext4, linux-xfs, ritesh.list, ojaswin, zlang,
	david

On Wed, Apr 09, 2025 at 07:00:52AM +0000, Nirjhar Roy (IBM) wrote:
> Replace exit <return-val> with _exit <return-val> which
> is introduced in the previous patch.
> 
> Signed-off-by: Nirjhar Roy (IBM) <nirjhar.roy.lists@gmail.com>
> Reviewed-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>
> Reviewed-by: Dave Chinner <dchinner@redhat.com>

Not sure why _filter_hole_fiemap gets whitespace cleanups in this patch,
but I don't care to make Nirjhar rev the whole series just for
whitespace so

Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

> ---
>  common/btrfs    |   6 +--
>  common/ceph     |   2 +-
>  common/config   |   7 ++--
>  common/dump     |  11 +++--
>  common/ext4     |   2 +-
>  common/populate |   2 +-
>  common/preamble |   2 +-
>  common/punch    |  13 +++---
>  common/rc       | 105 +++++++++++++++++++++++-------------------------
>  common/repair   |   4 +-
>  common/xfs      |   8 ++--
>  11 files changed, 78 insertions(+), 84 deletions(-)
> 
> diff --git a/common/btrfs b/common/btrfs
> index a3b9c12f..3725632c 100644
> --- a/common/btrfs
> +++ b/common/btrfs
> @@ -80,7 +80,7 @@ _require_btrfs_mkfs_feature()
>  {
>  	if [ -z $1 ]; then
>  		echo "Missing feature name argument for _require_btrfs_mkfs_feature"
> -		exit 1
> +		_exit 1
>  	fi
>  	feat=$1
>  	$MKFS_BTRFS_PROG -O list-all 2>&1 | \
> @@ -104,7 +104,7 @@ _require_btrfs_fs_feature()
>  {
>  	if [ -z $1 ]; then
>  		echo "Missing feature name argument for _require_btrfs_fs_feature"
> -		exit 1
> +		_exit 1
>  	fi
>  	feat=$1
>  	modprobe btrfs > /dev/null 2>&1
> @@ -214,7 +214,7 @@ _check_btrfs_filesystem()
>  	if [ $ok -eq 0 ]; then
>  		status=1
>  		if [ "$iam" != "check" ]; then
> -			exit 1
> +			_exit 1
>  		fi
>  		return 1
>  	fi
> diff --git a/common/ceph b/common/ceph
> index d6f24df1..df7a6814 100644
> --- a/common/ceph
> +++ b/common/ceph
> @@ -14,7 +14,7 @@ _ceph_create_file_layout()
>  
>  	if [ -e $fname ]; then
>  		echo "File $fname already exists."
> -		exit 1
> +		_exit 1
>  	fi
>  	touch $fname
>  	$SETFATTR_PROG -n ceph.file.layout \
> diff --git a/common/config b/common/config
> index 7dd78dbe..eada3971 100644
> --- a/common/config
> +++ b/common/config
> @@ -124,8 +124,7 @@ set_mkfs_prog_path_with_opts()
>  _fatal()
>  {
>      echo "$*"
> -    status=1
> -    exit 1
> +    _exit 1
>  }
>  
>  export MKFS_PROG="$(type -P mkfs)"
> @@ -869,7 +868,7 @@ get_next_config() {
>  		echo "Warning: need to define parameters for host $HOST"
>  		echo "       or set variables:"
>  		echo "       $MC"
> -		exit 1
> +		_exit 1
>  	fi
>  
>  	_check_device TEST_DEV required $TEST_DEV
> @@ -880,7 +879,7 @@ get_next_config() {
>  	if [ ! -z "$SCRATCH_DEV_POOL" ]; then
>  		if [ ! -z "$SCRATCH_DEV" ]; then
>  			echo "common/config: Error: \$SCRATCH_DEV ($SCRATCH_DEV) should be unset when \$SCRATCH_DEV_POOL ($SCRATCH_DEV_POOL) is set"
> -			exit 1
> +			_exit 1
>  		fi
>  		SCRATCH_DEV=`echo $SCRATCH_DEV_POOL | awk '{print $1}'`
>  		export SCRATCH_DEV
> diff --git a/common/dump b/common/dump
> index 6dcd6250..09859006 100644
> --- a/common/dump
> +++ b/common/dump
> @@ -280,8 +280,7 @@ _create_dumpdir_stress_num()
>      rm -rf $dump_dir
>      if ! mkdir $dump_dir; then
>          echo "    failed to mkdir $dump_dir"
> -        status=1
> -        exit
> +        _exit 1
>      fi
>  
>      # Remove fsstress commands that aren't supported on all xfs configs so that
> @@ -480,7 +479,7 @@ _do_create_dumpdir_fill()
>  		else
>  		    $verbose && echo
>  		    echo "Error: cannot mkdir \"$dir\""
> -		    exit 1
> +		    _exit 1
>  		fi
>  	    fi
>  	else
> @@ -496,7 +495,7 @@ _do_create_dumpdir_fill()
>  		    else
>  			$verbose && echo
>  			echo "Error: cannot mkdir \"$dir\""
> -			exit 1
> +			_exit 1
>  		    fi
>  		fi
>  	    fi
> @@ -507,7 +506,7 @@ _do_create_dumpdir_fill()
>  	    else
>  		$verbose && echo
>  		echo "Error: cannot create \"$file\""
> -		exit 1
> +		_exit 1
>  	    fi
>  	fi
>  	if [ -n "$owner" -a -n "$group" ]; then
> @@ -649,7 +648,7 @@ _do_create_dump_symlinks()
>  		else
>  		    $verbose && echo
>  		    echo "Error: cannot mkdir \"$dir\""
> -		    exit 1
> +		    _exit 1
>  		fi
>  	    fi
>  	fi
> diff --git a/common/ext4 b/common/ext4
> index e1b336d3..f88fa532 100644
> --- a/common/ext4
> +++ b/common/ext4
> @@ -182,7 +182,7 @@ _require_scratch_ext4_feature()
>  {
>      if [ -z "$1" ]; then
>          echo "Usage: _require_scratch_ext4_feature feature"
> -        exit 1
> +        _exit 1
>      fi
>      $MKFS_EXT4_PROG -F $MKFS_OPTIONS -O "$1" \
>  		    $SCRATCH_DEV 512m >/dev/null 2>&1 \
> diff --git a/common/populate b/common/populate
> index 7352f598..50dc75d3 100644
> --- a/common/populate
> +++ b/common/populate
> @@ -1003,7 +1003,7 @@ _fill_fs()
>  
>  	if [ $# -ne 4 ]; then
>  		echo "Usage: _fill_fs filesize dir blocksize switch_user"
> -		exit 1
> +		_exit 1
>  	fi
>  
>  	if [ $switch_user -eq 0 ]; then
> diff --git a/common/preamble b/common/preamble
> index c92e55bb..ba029a34 100644
> --- a/common/preamble
> +++ b/common/preamble
> @@ -35,7 +35,7 @@ _begin_fstest()
>  {
>  	if [ -n "$seq" ]; then
>  		echo "_begin_fstest can only be called once!"
> -		exit 1
> +		_exit 1
>  	fi
>  
>  	seq=`basename $0`
> diff --git a/common/punch b/common/punch
> index 43ccab69..64d665d8 100644
> --- a/common/punch
> +++ b/common/punch
> @@ -172,16 +172,16 @@ _filter_fiemap_flags()
>  	$AWK_PROG -e "$awk_script" | _coalesce_extents
>  }
>  
> -# Filters fiemap output to only print the 
> +# Filters fiemap output to only print the
>  # file offset column and whether or not
>  # it is an extent or a hole
>  _filter_hole_fiemap()
>  {
>  	$AWK_PROG '
>  		$3 ~ /hole/ {
> -			print $1, $2, $3; 
> +			print $1, $2, $3;
>  			next;
> -		}   
> +		}
>  		$5 ~ /0x[[:xdigit:]]+/ {
>  			print $1, $2, "extent";
>  		}' |
> @@ -224,8 +224,7 @@ _filter_bmap()
>  
>  die_now()
>  {
> -	status=1
> -	exit
> +	_exit 1
>  }
>  
>  # test the different corner cases for zeroing a range:
> @@ -276,7 +275,7 @@ _test_generic_punch()
>  		u)	unwritten_tests=
>  		;;
>  		?)	echo Invalid flag
> -		exit 1
> +		_exit 1
>  		;;
>  		esac
>  	done
> @@ -552,7 +551,7 @@ _test_block_boundaries()
>  		d)	sync_cmd=
>  		;;
>  		?)	echo Invalid flag
> -		exit 1
> +		_exit 1
>  		;;
>  		esac
>  	done
> diff --git a/common/rc b/common/rc
> index 038c22f6..3b21eb27 100644
> --- a/common/rc
> +++ b/common/rc
> @@ -909,8 +909,7 @@ _mkfs_dev()
>  	# output stored mkfs output
>  	cat $tmp.mkfserr >&2
>  	cat $tmp.mkfsstd
> -	status=1
> -	exit 1
> +	_exit 1
>      fi
>      rm -f $tmp.mkfserr $tmp.mkfsstd
>  }
> @@ -1575,7 +1574,7 @@ _get_pids_by_name()
>      if [ $# -ne 1 ]
>      then
>  	echo "Usage: _get_pids_by_name process-name" 1>&2
> -	exit 1
> +	_exit 1
>      fi
>  
>      # Algorithm ... all ps(1) variants have a time of the form MM:SS or
> @@ -1609,7 +1608,7 @@ _df_device()
>      if [ $# -ne 1 ]
>      then
>  	echo "Usage: _df_device device" 1>&2
> -	exit 1
> +	_exit 1
>      fi
>  
>      # Note that we use "==" here so awk doesn't try to interpret an NFS over
> @@ -1641,7 +1640,7 @@ _df_dir()
>      if [ $# -ne 1 ]
>      then
>  	echo "Usage: _df_dir device" 1>&2
> -	exit 1
> +	_exit 1
>      fi
>  
>      $DF_PROG $1 2>/dev/null | $AWK_PROG -v what=$1 '
> @@ -1667,7 +1666,7 @@ _used()
>      if [ $# -ne 1 ]
>      then
>  	echo "Usage: _used device" 1>&2
> -	exit 1
> +	_exit 1
>      fi
>  
>      _df_device $1 | $AWK_PROG '{ sub("%", "") ; print $6 }'
> @@ -1680,7 +1679,7 @@ _fs_type()
>      if [ $# -ne 1 ]
>      then
>  	echo "Usage: _fs_type device" 1>&2
> -	exit 1
> +	_exit 1
>      fi
>  
>      #
> @@ -1705,7 +1704,7 @@ _fs_options()
>      if [ $# -ne 1 ]
>      then
>  	echo "Usage: _fs_options device" 1>&2
> -	exit 1
> +	_exit 1
>      fi
>  
>      $AWK_PROG -v dev=$1 '
> @@ -1720,7 +1719,7 @@ _is_block_dev()
>      if [ $# -ne 1 ]
>      then
>  	echo "Usage: _is_block_dev dev" 1>&2
> -	exit 1
> +	_exit 1
>      fi
>  
>      local dev=$1
> @@ -1739,7 +1738,7 @@ _is_char_dev()
>  {
>  	if [ $# -ne 1 ]; then
>  		echo "Usage: _is_char_dev dev" 1>&2
> -		exit 1
> +		_exit 1
>  	fi
>  
>  	local dev=$1
> @@ -1772,7 +1771,7 @@ _do()
>  	echo -n "$note... "
>      else
>  	echo "Usage: _do [note] cmd" 1>&2
> -	status=1; exit
> +	_exit 1
>      fi
>  
>      (eval "echo '---' \"$cmd\"") >>$seqres.full
> @@ -1793,7 +1792,7 @@ _do()
>      then
>  	[ $# -ne 2 ] && echo
>  	eval "echo \"$cmd\" failed \(returned $ret\): see $seqres.full"
> -	status=1; exit
> +	_exit 1
>      fi
>  
>      return $ret
> @@ -1809,8 +1808,7 @@ _notrun()
>      rm -f ${RESULT_DIR}/require_test*
>      rm -f ${RESULT_DIR}/require_scratch*
>  
> -    status=0
> -    exit
> +    _exit 0
>  }
>  
>  # just plain bail out
> @@ -1819,8 +1817,7 @@ _fail()
>  {
>      echo "$*" | tee -a $seqres.full
>      echo "(see $seqres.full for details)"
> -    status=1
> -    exit 1
> +    _exit 1
>  }
>  
>  #
> @@ -2049,14 +2046,14 @@ _require_scratch_nocheck()
>  
>      _check_mounted_on SCRATCH_DEV $SCRATCH_DEV SCRATCH_MNT $SCRATCH_MNT
>      local err=$?
> -    [ $err -le 1 ] || exit 1
> +    [ $err -le 1 ] || _exit 1
>      if [ $err -eq 0 ]
>      then
>          # if it's mounted, unmount it
>          if ! _scratch_unmount
>          then
>              echo "failed to unmount $SCRATCH_DEV"
> -            exit 1
> +            _exit 1
>          fi
>      fi
>      rm -f ${RESULT_DIR}/require_scratch "$RESULT_DIR/.skip_orebuild" "$RESULT_DIR/.skip_rebuild"
> @@ -2273,13 +2270,13 @@ _require_test()
>  
>      _check_mounted_on TEST_DEV $TEST_DEV TEST_DIR $TEST_DIR
>      local err=$?
> -    [ $err -le 1 ] || exit 1
> +    [ $err -le 1 ] || _exit 1
>      if [ $err -ne 0 ]
>      then
>  	if ! _test_mount
>  	then
>  		echo "!!! failed to mount $TEST_DEV on $TEST_DIR"
> -		exit 1
> +		_exit 1
>  	fi
>      fi
>      touch ${RESULT_DIR}/require_test
> @@ -2391,7 +2388,7 @@ _require_block_device()
>  {
>  	if [ -z "$1" ]; then
>  		echo "Usage: _require_block_device <dev>" 1>&2
> -		exit 1
> +		_exit 1
>  	fi
>  	if [ "`_is_block_dev "$1"`" == "" ]; then
>  		_notrun "require $1 to be valid block disk"
> @@ -2404,7 +2401,7 @@ _require_local_device()
>  {
>  	if [ -z "$1" ]; then
>  		echo "Usage: _require_local_device <dev>" 1>&2
> -		exit 1
> +		_exit 1
>  	fi
>  	if [ "`_is_block_dev "$1"`" != "" ]; then
>  		return 0
> @@ -2512,7 +2509,7 @@ _zone_type()
>  	local target=$1
>  	if [ -z $target ]; then
>  		echo "Usage: _zone_type <device>"
> -		exit 1
> +		_exit 1
>  	fi
>  	local sdev=`_short_dev $target`
>  
> @@ -2528,7 +2525,7 @@ _require_zoned_device()
>  	local target=$1
>  	if [ -z $target ]; then
>  		echo "Usage: _require_zoned_device <device>"
> -		exit 1
> +		_exit 1
>  	fi
>  
>  	local type=`_zone_type ${target}`
> @@ -2668,7 +2665,7 @@ _run_aiodio()
>      if [ -z "$1" ]
>      then
>          echo "usage: _run_aiodio command_name" 2>&1
> -        status=1; exit 1
> +        _exit 1
>      fi
>  
>      _require_aiodio $1
> @@ -2880,7 +2877,7 @@ _require_xfs_io_command()
>  	if [ -z "$1" ]
>  	then
>  		echo "Usage: _require_xfs_io_command command [switch]" 1>&2
> -		exit 1
> +		_exit 1
>  	fi
>  	local command=$1
>  	shift
> @@ -3364,7 +3361,7 @@ _is_dev_mounted()
>  
>  	if [ $# -lt 1 ]; then
>  		echo "Usage: _is_dev_mounted <device> [fstype]" 1>&2
> -		exit 1
> +		_exit 1
>  	fi
>  
>  	findmnt -rncv -S $dev -t $fstype -o TARGET | head -1
> @@ -3378,7 +3375,7 @@ _is_dir_mountpoint()
>  
>  	if [ $# -lt 1 ]; then
>  		echo "Uasge: _is_dir_mountpoint <dir> [fstype]" 1>&2
> -		exit 1
> +		_exit 1
>  	fi
>  
>  	findmnt -rncv -t $fstype -o TARGET $dir | head -1
> @@ -3391,7 +3388,7 @@ _remount()
>      if [ $# -ne 2 ]
>      then
>  	echo "Usage: _remount device ro/rw" 1>&2
> -	exit 1
> +	_exit 1
>      fi
>      local device=$1
>      local mode=$2
> @@ -3399,7 +3396,7 @@ _remount()
>      if ! mount -o remount,$mode $device
>      then
>          echo "_remount: failed to remount filesystem on $device as $mode"
> -        exit 1
> +        _exit 1
>      fi
>  }
>  
> @@ -3417,7 +3414,7 @@ _umount_or_remount_ro()
>      if [ $# -ne 1 ]
>      then
>  	echo "Usage: _umount_or_remount_ro <device>" 1>&2
> -	exit 1
> +	_exit 1
>      fi
>  
>      local device=$1
> @@ -3435,7 +3432,7 @@ _mount_or_remount_rw()
>  {
>  	if [ $# -ne 3 ]; then
>  		echo "Usage: _mount_or_remount_rw <opts> <dev> <mnt>" 1>&2
> -		exit 1
> +		_exit 1
>  	fi
>  	local mount_opts=$1
>  	local device=$2
> @@ -3516,7 +3513,7 @@ _check_generic_filesystem()
>      if [ $ok -eq 0 ]; then
>  	status=1
>  	if [ "$iam" != "check" ]; then
> -		exit 1
> +		_exit 1
>  	fi
>  	return 1
>      fi
> @@ -3582,7 +3579,7 @@ _check_udf_filesystem()
>      if [ $# -ne 1 -a $# -ne 2 ]
>      then
>  	echo "Usage: _check_udf_filesystem device [last_block]" 1>&2
> -	exit 1
> +	_exit 1
>      fi
>  
>      if [ ! -x $here/src/udf_test ]
> @@ -3776,7 +3773,7 @@ _get_os_name()
>  		echo 'linux'
>  	else
>  		echo Unknown operating system: `uname`
> -		exit
> +		_exit 1
>  	fi
>  }
>  
> @@ -3837,7 +3834,7 @@ _link_out_file()
>  _die()
>  {
>          echo $@
> -        exit 1
> +        _exit 1
>  }
>  
>  # convert urandom incompressible data to compressible text data
> @@ -3994,7 +3991,7 @@ _require_scratch_dev_pool()
>  		if _mount | grep -q $i; then
>  			if ! _unmount $i; then
>  		            echo "failed to unmount $i - aborting"
> -		            exit 1
> +		            _exit 1
>  		        fi
>  		fi
>  		# To help better debug when something fails, we remove
> @@ -4403,7 +4400,7 @@ _require_batched_discard()
>  {
>  	if [ $# -ne 1 ]; then
>  		echo "Usage: _require_batched_discard mnt_point" 1>&2
> -		exit 1
> +		_exit 1
>  	fi
>  	_require_fstrim
>  
> @@ -4630,7 +4627,7 @@ _require_chattr()
>  {
>  	if [ -z "$1" ]; then
>  		echo "Usage: _require_chattr <attr>"
> -		exit 1
> +		_exit 1
>  	fi
>  	local attribute=$1
>  
> @@ -4649,7 +4646,7 @@ _get_total_inode()
>  {
>  	if [ -z "$1" ]; then
>  		echo "Usage: _get_total_inode <mnt>"
> -		exit 1
> +		_exit 1
>  	fi
>  	local nr_inode;
>  	nr_inode=`$DF_PROG -i $1 | tail -1 | awk '{print $3}'`
> @@ -4660,7 +4657,7 @@ _get_used_inode()
>  {
>  	if [ -z "$1" ]; then
>  		echo "Usage: _get_used_inode <mnt>"
> -		exit 1
> +		_exit 1
>  	fi
>  	local nr_inode;
>  	nr_inode=`$DF_PROG -i $1 | tail -1 | awk '{print $4}'`
> @@ -4671,7 +4668,7 @@ _get_used_inode_percent()
>  {
>  	if [ -z "$1" ]; then
>  		echo "Usage: _get_used_inode_percent <mnt>"
> -		exit 1
> +		_exit 1
>  	fi
>  	local pct_inode;
>  	pct_inode=`$DF_PROG -i $1 | tail -1 | awk '{ print $6 }' | \
> @@ -4683,7 +4680,7 @@ _get_free_inode()
>  {
>  	if [ -z "$1" ]; then
>  		echo "Usage: _get_free_inode <mnt>"
> -		exit 1
> +		_exit 1
>  	fi
>  	local nr_inode;
>  	nr_inode=`$DF_PROG -i $1 | tail -1 | awk '{print $5}'`
> @@ -4696,7 +4693,7 @@ _get_available_space()
>  {
>  	if [ -z "$1" ]; then
>  		echo "Usage: _get_available_space <mnt>"
> -		exit 1
> +		_exit 1
>  	fi
>  	$DF_PROG -B 1 $1 | tail -n1 | awk '{ print $5 }'
>  }
> @@ -4707,7 +4704,7 @@ _get_total_space()
>  {
>  	if [ -z "$1" ]; then
>  		echo "Usage: _get_total_space <mnt>"
> -		exit 1
> +		_exit 1
>  	fi
>  	$DF_PROG -B 1 $1 | tail -n1 | awk '{ print $3 }'
>  }
> @@ -4952,7 +4949,7 @@ init_rc()
>  	if [ "$TEST_DEV" = ""  ]
>  	then
>  		echo "common/rc: Error: \$TEST_DEV is not set"
> -		exit 1
> +		_exit 1
>  	fi
>  
>  	# if $TEST_DEV is not mounted, mount it now as XFS
> @@ -4966,20 +4963,20 @@ init_rc()
>  			if ! _test_mount
>  			then
>  				echo "common/rc: could not mount $TEST_DEV on $TEST_DIR"
> -				exit 1
> +				_exit 1
>  			fi
>  		fi
>  	fi
>  
>  	# Sanity check that TEST partition is not mounted at another mount point
>  	# or as another fs type
> -	_check_mounted_on TEST_DEV $TEST_DEV TEST_DIR $TEST_DIR $FSTYP || exit 1
> +	_check_mounted_on TEST_DEV $TEST_DEV TEST_DIR $TEST_DIR $FSTYP || _exit 1
>  	if [ -n "$SCRATCH_DEV" ]; then
>  		# Sanity check that SCRATCH partition is not mounted at another
>  		# mount point, because it is about to be unmounted and formatted.
>  		# Another fs type for scratch is fine (bye bye old fs type).
>  		_check_mounted_on SCRATCH_DEV $SCRATCH_DEV SCRATCH_MNT $SCRATCH_MNT
> -		[ $? -le 1 ] || exit 1
> +		[ $? -le 1 ] || _exit 1
>  	fi
>  
>  	# Figure out if we need to add -F ("foreign", deprecated) option to xfs_io
> @@ -5029,7 +5026,7 @@ _get_file_block_size()
>  {
>  	if [ -z $1 ] || [ ! -d $1 ]; then
>  		echo "Missing mount point argument for _get_file_block_size"
> -		exit 1
> +		_exit 1
>  	fi
>  
>  	case "$FSTYP" in
> @@ -5076,7 +5073,7 @@ _get_block_size()
>  {
>  	if [ -z $1 ] || [ ! -d $1 ]; then
>  		echo "Missing mount point argument for _get_block_size"
> -		exit 1
> +		_exit 1
>  	fi
>  	stat -f -c %S $1
>  }
> @@ -5146,14 +5143,14 @@ _run_hugepage_fsx() {
>  	fi
>  	cat $tmp.hugepage_fsx
>  	rm -f $tmp.hugepage_fsx
> -	test $res -ne 0 && exit 1
> +	test $res -ne 0 && _exit 1
>  	return 0
>  }
>  
>  # run fsx or exit the test
>  run_fsx()
>  {
> -	_run_fsx "$@" || exit 1
> +	_run_fsx "$@" || _exit 1
>  }
>  
>  _require_statx()
> @@ -5318,7 +5315,7 @@ _get_max_file_size()
>  {
>  	if [ -z $1 ] || [ ! -d $1 ]; then
>  		echo "Missing mount point argument for _get_max_file_size"
> -		exit 1
> +		_exit 1
>  	fi
>  
>  	local mnt=$1
> diff --git a/common/repair b/common/repair
> index a79f9b2b..fd206f8e 100644
> --- a/common/repair
> +++ b/common/repair
> @@ -16,7 +16,7 @@ _zero_position()
>  		}'`
>  	if [ -z "$offset" -o -z "$length" ]; then
>  		echo "cannot calculate offset ($offset) or length ($length)"
> -		exit
> +		_exit 1
>  	fi
>  	length=`expr $length / 512`
>  	$here/src/devzero -v $value -b 1 -n $length -o $offset $SCRATCH_DEV \
> @@ -113,7 +113,7 @@ _filter_dd()
>  }
>  
>  # do some controlled corrupting & ensure repair recovers us
> -# 
> +#
>  _check_repair()
>  {
>  	value=$1
> diff --git a/common/xfs b/common/xfs
> index 81d568d3..96c15f3c 100644
> --- a/common/xfs
> +++ b/common/xfs
> @@ -553,7 +553,7 @@ _require_xfs_db_command()
>  {
>  	if [ $# -ne 1 ]; then
>  		echo "Usage: _require_xfs_db_command command" 1>&2
> -		exit 1
> +		_exit 1
>  	fi
>  	command=$1
>  
> @@ -789,7 +789,7 @@ _check_xfs_filesystem()
>  
>  	if [ $# -ne 3 ]; then
>  		echo "Usage: _check_xfs_filesystem device <logdev>|none <rtdev>|none" 1>&2
> -		exit 1
> +		_exit 1
>  	fi
>  
>  	extra_mount_options=""
> @@ -1014,7 +1014,7 @@ _check_xfs_filesystem()
>  	if [ $ok -eq 0 ]; then
>  		status=1
>  		if [ "$iam" != "check" ]; then
> -			exit 1
> +			_exit 1
>  		fi
>  		return 1
>  	fi
> @@ -1379,7 +1379,7 @@ _require_xfs_spaceman_command()
>  {
>  	if [ -z "$1" ]; then
>  		echo "Usage: _require_xfs_spaceman_command command [switch]" 1>&2
> -		exit 1
> +		_exit 1
>  	fi
>  	local command=$1
>  	shift
> -- 
> 2.34.1
> 
> 

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v4 6/6] common: exit --> _exit
  2025-04-09 15:24   ` Darrick J. Wong
@ 2025-04-09 16:29     ` Nirjhar Roy (IBM)
  2025-04-10 14:31     ` Zorro Lang
  1 sibling, 0 replies; 15+ messages in thread
From: Nirjhar Roy (IBM) @ 2025-04-09 16:29 UTC (permalink / raw)
  To: Darrick J. Wong
  Cc: fstests, linux-ext4, linux-xfs, ritesh.list, ojaswin, zlang,
	david


On 4/9/25 20:54, Darrick J. Wong wrote:
> On Wed, Apr 09, 2025 at 07:00:52AM +0000, Nirjhar Roy (IBM) wrote:
>> Replace exit <return-val> with _exit <return-val> which
>> is introduced in the previous patch.
>>
>> Signed-off-by: Nirjhar Roy (IBM) <nirjhar.roy.lists@gmail.com>
>> Reviewed-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>
>> Reviewed-by: Dave Chinner <dchinner@redhat.com>
> Not sure why _filter_hole_fiemap gets whitespace cleanups in this patch,
> but I don't care to make Nirjhar rev the whole series just for
> whitespace so

Thank you Darrick for reviewing all the patches in the series.

--NR

>
> Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
>
> --D
>
>> ---
>>   common/btrfs    |   6 +--
>>   common/ceph     |   2 +-
>>   common/config   |   7 ++--
>>   common/dump     |  11 +++--
>>   common/ext4     |   2 +-
>>   common/populate |   2 +-
>>   common/preamble |   2 +-
>>   common/punch    |  13 +++---
>>   common/rc       | 105 +++++++++++++++++++++++-------------------------
>>   common/repair   |   4 +-
>>   common/xfs      |   8 ++--
>>   11 files changed, 78 insertions(+), 84 deletions(-)
>>
>> diff --git a/common/btrfs b/common/btrfs
>> index a3b9c12f..3725632c 100644
>> --- a/common/btrfs
>> +++ b/common/btrfs
>> @@ -80,7 +80,7 @@ _require_btrfs_mkfs_feature()
>>   {
>>   	if [ -z $1 ]; then
>>   		echo "Missing feature name argument for _require_btrfs_mkfs_feature"
>> -		exit 1
>> +		_exit 1
>>   	fi
>>   	feat=$1
>>   	$MKFS_BTRFS_PROG -O list-all 2>&1 | \
>> @@ -104,7 +104,7 @@ _require_btrfs_fs_feature()
>>   {
>>   	if [ -z $1 ]; then
>>   		echo "Missing feature name argument for _require_btrfs_fs_feature"
>> -		exit 1
>> +		_exit 1
>>   	fi
>>   	feat=$1
>>   	modprobe btrfs > /dev/null 2>&1
>> @@ -214,7 +214,7 @@ _check_btrfs_filesystem()
>>   	if [ $ok -eq 0 ]; then
>>   		status=1
>>   		if [ "$iam" != "check" ]; then
>> -			exit 1
>> +			_exit 1
>>   		fi
>>   		return 1
>>   	fi
>> diff --git a/common/ceph b/common/ceph
>> index d6f24df1..df7a6814 100644
>> --- a/common/ceph
>> +++ b/common/ceph
>> @@ -14,7 +14,7 @@ _ceph_create_file_layout()
>>   
>>   	if [ -e $fname ]; then
>>   		echo "File $fname already exists."
>> -		exit 1
>> +		_exit 1
>>   	fi
>>   	touch $fname
>>   	$SETFATTR_PROG -n ceph.file.layout \
>> diff --git a/common/config b/common/config
>> index 7dd78dbe..eada3971 100644
>> --- a/common/config
>> +++ b/common/config
>> @@ -124,8 +124,7 @@ set_mkfs_prog_path_with_opts()
>>   _fatal()
>>   {
>>       echo "$*"
>> -    status=1
>> -    exit 1
>> +    _exit 1
>>   }
>>   
>>   export MKFS_PROG="$(type -P mkfs)"
>> @@ -869,7 +868,7 @@ get_next_config() {
>>   		echo "Warning: need to define parameters for host $HOST"
>>   		echo "       or set variables:"
>>   		echo "       $MC"
>> -		exit 1
>> +		_exit 1
>>   	fi
>>   
>>   	_check_device TEST_DEV required $TEST_DEV
>> @@ -880,7 +879,7 @@ get_next_config() {
>>   	if [ ! -z "$SCRATCH_DEV_POOL" ]; then
>>   		if [ ! -z "$SCRATCH_DEV" ]; then
>>   			echo "common/config: Error: \$SCRATCH_DEV ($SCRATCH_DEV) should be unset when \$SCRATCH_DEV_POOL ($SCRATCH_DEV_POOL) is set"
>> -			exit 1
>> +			_exit 1
>>   		fi
>>   		SCRATCH_DEV=`echo $SCRATCH_DEV_POOL | awk '{print $1}'`
>>   		export SCRATCH_DEV
>> diff --git a/common/dump b/common/dump
>> index 6dcd6250..09859006 100644
>> --- a/common/dump
>> +++ b/common/dump
>> @@ -280,8 +280,7 @@ _create_dumpdir_stress_num()
>>       rm -rf $dump_dir
>>       if ! mkdir $dump_dir; then
>>           echo "    failed to mkdir $dump_dir"
>> -        status=1
>> -        exit
>> +        _exit 1
>>       fi
>>   
>>       # Remove fsstress commands that aren't supported on all xfs configs so that
>> @@ -480,7 +479,7 @@ _do_create_dumpdir_fill()
>>   		else
>>   		    $verbose && echo
>>   		    echo "Error: cannot mkdir \"$dir\""
>> -		    exit 1
>> +		    _exit 1
>>   		fi
>>   	    fi
>>   	else
>> @@ -496,7 +495,7 @@ _do_create_dumpdir_fill()
>>   		    else
>>   			$verbose && echo
>>   			echo "Error: cannot mkdir \"$dir\""
>> -			exit 1
>> +			_exit 1
>>   		    fi
>>   		fi
>>   	    fi
>> @@ -507,7 +506,7 @@ _do_create_dumpdir_fill()
>>   	    else
>>   		$verbose && echo
>>   		echo "Error: cannot create \"$file\""
>> -		exit 1
>> +		_exit 1
>>   	    fi
>>   	fi
>>   	if [ -n "$owner" -a -n "$group" ]; then
>> @@ -649,7 +648,7 @@ _do_create_dump_symlinks()
>>   		else
>>   		    $verbose && echo
>>   		    echo "Error: cannot mkdir \"$dir\""
>> -		    exit 1
>> +		    _exit 1
>>   		fi
>>   	    fi
>>   	fi
>> diff --git a/common/ext4 b/common/ext4
>> index e1b336d3..f88fa532 100644
>> --- a/common/ext4
>> +++ b/common/ext4
>> @@ -182,7 +182,7 @@ _require_scratch_ext4_feature()
>>   {
>>       if [ -z "$1" ]; then
>>           echo "Usage: _require_scratch_ext4_feature feature"
>> -        exit 1
>> +        _exit 1
>>       fi
>>       $MKFS_EXT4_PROG -F $MKFS_OPTIONS -O "$1" \
>>   		    $SCRATCH_DEV 512m >/dev/null 2>&1 \
>> diff --git a/common/populate b/common/populate
>> index 7352f598..50dc75d3 100644
>> --- a/common/populate
>> +++ b/common/populate
>> @@ -1003,7 +1003,7 @@ _fill_fs()
>>   
>>   	if [ $# -ne 4 ]; then
>>   		echo "Usage: _fill_fs filesize dir blocksize switch_user"
>> -		exit 1
>> +		_exit 1
>>   	fi
>>   
>>   	if [ $switch_user -eq 0 ]; then
>> diff --git a/common/preamble b/common/preamble
>> index c92e55bb..ba029a34 100644
>> --- a/common/preamble
>> +++ b/common/preamble
>> @@ -35,7 +35,7 @@ _begin_fstest()
>>   {
>>   	if [ -n "$seq" ]; then
>>   		echo "_begin_fstest can only be called once!"
>> -		exit 1
>> +		_exit 1
>>   	fi
>>   
>>   	seq=`basename $0`
>> diff --git a/common/punch b/common/punch
>> index 43ccab69..64d665d8 100644
>> --- a/common/punch
>> +++ b/common/punch
>> @@ -172,16 +172,16 @@ _filter_fiemap_flags()
>>   	$AWK_PROG -e "$awk_script" | _coalesce_extents
>>   }
>>   
>> -# Filters fiemap output to only print the
>> +# Filters fiemap output to only print the
>>   # file offset column and whether or not
>>   # it is an extent or a hole
>>   _filter_hole_fiemap()
>>   {
>>   	$AWK_PROG '
>>   		$3 ~ /hole/ {
>> -			print $1, $2, $3;
>> +			print $1, $2, $3;
>>   			next;
>> -		}
>> +		}
>>   		$5 ~ /0x[[:xdigit:]]+/ {
>>   			print $1, $2, "extent";
>>   		}' |
>> @@ -224,8 +224,7 @@ _filter_bmap()
>>   
>>   die_now()
>>   {
>> -	status=1
>> -	exit
>> +	_exit 1
>>   }
>>   
>>   # test the different corner cases for zeroing a range:
>> @@ -276,7 +275,7 @@ _test_generic_punch()
>>   		u)	unwritten_tests=
>>   		;;
>>   		?)	echo Invalid flag
>> -		exit 1
>> +		_exit 1
>>   		;;
>>   		esac
>>   	done
>> @@ -552,7 +551,7 @@ _test_block_boundaries()
>>   		d)	sync_cmd=
>>   		;;
>>   		?)	echo Invalid flag
>> -		exit 1
>> +		_exit 1
>>   		;;
>>   		esac
>>   	done
>> diff --git a/common/rc b/common/rc
>> index 038c22f6..3b21eb27 100644
>> --- a/common/rc
>> +++ b/common/rc
>> @@ -909,8 +909,7 @@ _mkfs_dev()
>>   	# output stored mkfs output
>>   	cat $tmp.mkfserr >&2
>>   	cat $tmp.mkfsstd
>> -	status=1
>> -	exit 1
>> +	_exit 1
>>       fi
>>       rm -f $tmp.mkfserr $tmp.mkfsstd
>>   }
>> @@ -1575,7 +1574,7 @@ _get_pids_by_name()
>>       if [ $# -ne 1 ]
>>       then
>>   	echo "Usage: _get_pids_by_name process-name" 1>&2
>> -	exit 1
>> +	_exit 1
>>       fi
>>   
>>       # Algorithm ... all ps(1) variants have a time of the form MM:SS or
>> @@ -1609,7 +1608,7 @@ _df_device()
>>       if [ $# -ne 1 ]
>>       then
>>   	echo "Usage: _df_device device" 1>&2
>> -	exit 1
>> +	_exit 1
>>       fi
>>   
>>       # Note that we use "==" here so awk doesn't try to interpret an NFS over
>> @@ -1641,7 +1640,7 @@ _df_dir()
>>       if [ $# -ne 1 ]
>>       then
>>   	echo "Usage: _df_dir device" 1>&2
>> -	exit 1
>> +	_exit 1
>>       fi
>>   
>>       $DF_PROG $1 2>/dev/null | $AWK_PROG -v what=$1 '
>> @@ -1667,7 +1666,7 @@ _used()
>>       if [ $# -ne 1 ]
>>       then
>>   	echo "Usage: _used device" 1>&2
>> -	exit 1
>> +	_exit 1
>>       fi
>>   
>>       _df_device $1 | $AWK_PROG '{ sub("%", "") ; print $6 }'
>> @@ -1680,7 +1679,7 @@ _fs_type()
>>       if [ $# -ne 1 ]
>>       then
>>   	echo "Usage: _fs_type device" 1>&2
>> -	exit 1
>> +	_exit 1
>>       fi
>>   
>>       #
>> @@ -1705,7 +1704,7 @@ _fs_options()
>>       if [ $# -ne 1 ]
>>       then
>>   	echo "Usage: _fs_options device" 1>&2
>> -	exit 1
>> +	_exit 1
>>       fi
>>   
>>       $AWK_PROG -v dev=$1 '
>> @@ -1720,7 +1719,7 @@ _is_block_dev()
>>       if [ $# -ne 1 ]
>>       then
>>   	echo "Usage: _is_block_dev dev" 1>&2
>> -	exit 1
>> +	_exit 1
>>       fi
>>   
>>       local dev=$1
>> @@ -1739,7 +1738,7 @@ _is_char_dev()
>>   {
>>   	if [ $# -ne 1 ]; then
>>   		echo "Usage: _is_char_dev dev" 1>&2
>> -		exit 1
>> +		_exit 1
>>   	fi
>>   
>>   	local dev=$1
>> @@ -1772,7 +1771,7 @@ _do()
>>   	echo -n "$note... "
>>       else
>>   	echo "Usage: _do [note] cmd" 1>&2
>> -	status=1; exit
>> +	_exit 1
>>       fi
>>   
>>       (eval "echo '---' \"$cmd\"") >>$seqres.full
>> @@ -1793,7 +1792,7 @@ _do()
>>       then
>>   	[ $# -ne 2 ] && echo
>>   	eval "echo \"$cmd\" failed \(returned $ret\): see $seqres.full"
>> -	status=1; exit
>> +	_exit 1
>>       fi
>>   
>>       return $ret
>> @@ -1809,8 +1808,7 @@ _notrun()
>>       rm -f ${RESULT_DIR}/require_test*
>>       rm -f ${RESULT_DIR}/require_scratch*
>>   
>> -    status=0
>> -    exit
>> +    _exit 0
>>   }
>>   
>>   # just plain bail out
>> @@ -1819,8 +1817,7 @@ _fail()
>>   {
>>       echo "$*" | tee -a $seqres.full
>>       echo "(see $seqres.full for details)"
>> -    status=1
>> -    exit 1
>> +    _exit 1
>>   }
>>   
>>   #
>> @@ -2049,14 +2046,14 @@ _require_scratch_nocheck()
>>   
>>       _check_mounted_on SCRATCH_DEV $SCRATCH_DEV SCRATCH_MNT $SCRATCH_MNT
>>       local err=$?
>> -    [ $err -le 1 ] || exit 1
>> +    [ $err -le 1 ] || _exit 1
>>       if [ $err -eq 0 ]
>>       then
>>           # if it's mounted, unmount it
>>           if ! _scratch_unmount
>>           then
>>               echo "failed to unmount $SCRATCH_DEV"
>> -            exit 1
>> +            _exit 1
>>           fi
>>       fi
>>       rm -f ${RESULT_DIR}/require_scratch "$RESULT_DIR/.skip_orebuild" "$RESULT_DIR/.skip_rebuild"
>> @@ -2273,13 +2270,13 @@ _require_test()
>>   
>>       _check_mounted_on TEST_DEV $TEST_DEV TEST_DIR $TEST_DIR
>>       local err=$?
>> -    [ $err -le 1 ] || exit 1
>> +    [ $err -le 1 ] || _exit 1
>>       if [ $err -ne 0 ]
>>       then
>>   	if ! _test_mount
>>   	then
>>   		echo "!!! failed to mount $TEST_DEV on $TEST_DIR"
>> -		exit 1
>> +		_exit 1
>>   	fi
>>       fi
>>       touch ${RESULT_DIR}/require_test
>> @@ -2391,7 +2388,7 @@ _require_block_device()
>>   {
>>   	if [ -z "$1" ]; then
>>   		echo "Usage: _require_block_device <dev>" 1>&2
>> -		exit 1
>> +		_exit 1
>>   	fi
>>   	if [ "`_is_block_dev "$1"`" == "" ]; then
>>   		_notrun "require $1 to be valid block disk"
>> @@ -2404,7 +2401,7 @@ _require_local_device()
>>   {
>>   	if [ -z "$1" ]; then
>>   		echo "Usage: _require_local_device <dev>" 1>&2
>> -		exit 1
>> +		_exit 1
>>   	fi
>>   	if [ "`_is_block_dev "$1"`" != "" ]; then
>>   		return 0
>> @@ -2512,7 +2509,7 @@ _zone_type()
>>   	local target=$1
>>   	if [ -z $target ]; then
>>   		echo "Usage: _zone_type <device>"
>> -		exit 1
>> +		_exit 1
>>   	fi
>>   	local sdev=`_short_dev $target`
>>   
>> @@ -2528,7 +2525,7 @@ _require_zoned_device()
>>   	local target=$1
>>   	if [ -z $target ]; then
>>   		echo "Usage: _require_zoned_device <device>"
>> -		exit 1
>> +		_exit 1
>>   	fi
>>   
>>   	local type=`_zone_type ${target}`
>> @@ -2668,7 +2665,7 @@ _run_aiodio()
>>       if [ -z "$1" ]
>>       then
>>           echo "usage: _run_aiodio command_name" 2>&1
>> -        status=1; exit 1
>> +        _exit 1
>>       fi
>>   
>>       _require_aiodio $1
>> @@ -2880,7 +2877,7 @@ _require_xfs_io_command()
>>   	if [ -z "$1" ]
>>   	then
>>   		echo "Usage: _require_xfs_io_command command [switch]" 1>&2
>> -		exit 1
>> +		_exit 1
>>   	fi
>>   	local command=$1
>>   	shift
>> @@ -3364,7 +3361,7 @@ _is_dev_mounted()
>>   
>>   	if [ $# -lt 1 ]; then
>>   		echo "Usage: _is_dev_mounted <device> [fstype]" 1>&2
>> -		exit 1
>> +		_exit 1
>>   	fi
>>   
>>   	findmnt -rncv -S $dev -t $fstype -o TARGET | head -1
>> @@ -3378,7 +3375,7 @@ _is_dir_mountpoint()
>>   
>>   	if [ $# -lt 1 ]; then
>>   		echo "Uasge: _is_dir_mountpoint <dir> [fstype]" 1>&2
>> -		exit 1
>> +		_exit 1
>>   	fi
>>   
>>   	findmnt -rncv -t $fstype -o TARGET $dir | head -1
>> @@ -3391,7 +3388,7 @@ _remount()
>>       if [ $# -ne 2 ]
>>       then
>>   	echo "Usage: _remount device ro/rw" 1>&2
>> -	exit 1
>> +	_exit 1
>>       fi
>>       local device=$1
>>       local mode=$2
>> @@ -3399,7 +3396,7 @@ _remount()
>>       if ! mount -o remount,$mode $device
>>       then
>>           echo "_remount: failed to remount filesystem on $device as $mode"
>> -        exit 1
>> +        _exit 1
>>       fi
>>   }
>>   
>> @@ -3417,7 +3414,7 @@ _umount_or_remount_ro()
>>       if [ $# -ne 1 ]
>>       then
>>   	echo "Usage: _umount_or_remount_ro <device>" 1>&2
>> -	exit 1
>> +	_exit 1
>>       fi
>>   
>>       local device=$1
>> @@ -3435,7 +3432,7 @@ _mount_or_remount_rw()
>>   {
>>   	if [ $# -ne 3 ]; then
>>   		echo "Usage: _mount_or_remount_rw <opts> <dev> <mnt>" 1>&2
>> -		exit 1
>> +		_exit 1
>>   	fi
>>   	local mount_opts=$1
>>   	local device=$2
>> @@ -3516,7 +3513,7 @@ _check_generic_filesystem()
>>       if [ $ok -eq 0 ]; then
>>   	status=1
>>   	if [ "$iam" != "check" ]; then
>> -		exit 1
>> +		_exit 1
>>   	fi
>>   	return 1
>>       fi
>> @@ -3582,7 +3579,7 @@ _check_udf_filesystem()
>>       if [ $# -ne 1 -a $# -ne 2 ]
>>       then
>>   	echo "Usage: _check_udf_filesystem device [last_block]" 1>&2
>> -	exit 1
>> +	_exit 1
>>       fi
>>   
>>       if [ ! -x $here/src/udf_test ]
>> @@ -3776,7 +3773,7 @@ _get_os_name()
>>   		echo 'linux'
>>   	else
>>   		echo Unknown operating system: `uname`
>> -		exit
>> +		_exit 1
>>   	fi
>>   }
>>   
>> @@ -3837,7 +3834,7 @@ _link_out_file()
>>   _die()
>>   {
>>           echo $@
>> -        exit 1
>> +        _exit 1
>>   }
>>   
>>   # convert urandom incompressible data to compressible text data
>> @@ -3994,7 +3991,7 @@ _require_scratch_dev_pool()
>>   		if _mount | grep -q $i; then
>>   			if ! _unmount $i; then
>>   		            echo "failed to unmount $i - aborting"
>> -		            exit 1
>> +		            _exit 1
>>   		        fi
>>   		fi
>>   		# To help better debug when something fails, we remove
>> @@ -4403,7 +4400,7 @@ _require_batched_discard()
>>   {
>>   	if [ $# -ne 1 ]; then
>>   		echo "Usage: _require_batched_discard mnt_point" 1>&2
>> -		exit 1
>> +		_exit 1
>>   	fi
>>   	_require_fstrim
>>   
>> @@ -4630,7 +4627,7 @@ _require_chattr()
>>   {
>>   	if [ -z "$1" ]; then
>>   		echo "Usage: _require_chattr <attr>"
>> -		exit 1
>> +		_exit 1
>>   	fi
>>   	local attribute=$1
>>   
>> @@ -4649,7 +4646,7 @@ _get_total_inode()
>>   {
>>   	if [ -z "$1" ]; then
>>   		echo "Usage: _get_total_inode <mnt>"
>> -		exit 1
>> +		_exit 1
>>   	fi
>>   	local nr_inode;
>>   	nr_inode=`$DF_PROG -i $1 | tail -1 | awk '{print $3}'`
>> @@ -4660,7 +4657,7 @@ _get_used_inode()
>>   {
>>   	if [ -z "$1" ]; then
>>   		echo "Usage: _get_used_inode <mnt>"
>> -		exit 1
>> +		_exit 1
>>   	fi
>>   	local nr_inode;
>>   	nr_inode=`$DF_PROG -i $1 | tail -1 | awk '{print $4}'`
>> @@ -4671,7 +4668,7 @@ _get_used_inode_percent()
>>   {
>>   	if [ -z "$1" ]; then
>>   		echo "Usage: _get_used_inode_percent <mnt>"
>> -		exit 1
>> +		_exit 1
>>   	fi
>>   	local pct_inode;
>>   	pct_inode=`$DF_PROG -i $1 | tail -1 | awk '{ print $6 }' | \
>> @@ -4683,7 +4680,7 @@ _get_free_inode()
>>   {
>>   	if [ -z "$1" ]; then
>>   		echo "Usage: _get_free_inode <mnt>"
>> -		exit 1
>> +		_exit 1
>>   	fi
>>   	local nr_inode;
>>   	nr_inode=`$DF_PROG -i $1 | tail -1 | awk '{print $5}'`
>> @@ -4696,7 +4693,7 @@ _get_available_space()
>>   {
>>   	if [ -z "$1" ]; then
>>   		echo "Usage: _get_available_space <mnt>"
>> -		exit 1
>> +		_exit 1
>>   	fi
>>   	$DF_PROG -B 1 $1 | tail -n1 | awk '{ print $5 }'
>>   }
>> @@ -4707,7 +4704,7 @@ _get_total_space()
>>   {
>>   	if [ -z "$1" ]; then
>>   		echo "Usage: _get_total_space <mnt>"
>> -		exit 1
>> +		_exit 1
>>   	fi
>>   	$DF_PROG -B 1 $1 | tail -n1 | awk '{ print $3 }'
>>   }
>> @@ -4952,7 +4949,7 @@ init_rc()
>>   	if [ "$TEST_DEV" = ""  ]
>>   	then
>>   		echo "common/rc: Error: \$TEST_DEV is not set"
>> -		exit 1
>> +		_exit 1
>>   	fi
>>   
>>   	# if $TEST_DEV is not mounted, mount it now as XFS
>> @@ -4966,20 +4963,20 @@ init_rc()
>>   			if ! _test_mount
>>   			then
>>   				echo "common/rc: could not mount $TEST_DEV on $TEST_DIR"
>> -				exit 1
>> +				_exit 1
>>   			fi
>>   		fi
>>   	fi
>>   
>>   	# Sanity check that TEST partition is not mounted at another mount point
>>   	# or as another fs type
>> -	_check_mounted_on TEST_DEV $TEST_DEV TEST_DIR $TEST_DIR $FSTYP || exit 1
>> +	_check_mounted_on TEST_DEV $TEST_DEV TEST_DIR $TEST_DIR $FSTYP || _exit 1
>>   	if [ -n "$SCRATCH_DEV" ]; then
>>   		# Sanity check that SCRATCH partition is not mounted at another
>>   		# mount point, because it is about to be unmounted and formatted.
>>   		# Another fs type for scratch is fine (bye bye old fs type).
>>   		_check_mounted_on SCRATCH_DEV $SCRATCH_DEV SCRATCH_MNT $SCRATCH_MNT
>> -		[ $? -le 1 ] || exit 1
>> +		[ $? -le 1 ] || _exit 1
>>   	fi
>>   
>>   	# Figure out if we need to add -F ("foreign", deprecated) option to xfs_io
>> @@ -5029,7 +5026,7 @@ _get_file_block_size()
>>   {
>>   	if [ -z $1 ] || [ ! -d $1 ]; then
>>   		echo "Missing mount point argument for _get_file_block_size"
>> -		exit 1
>> +		_exit 1
>>   	fi
>>   
>>   	case "$FSTYP" in
>> @@ -5076,7 +5073,7 @@ _get_block_size()
>>   {
>>   	if [ -z $1 ] || [ ! -d $1 ]; then
>>   		echo "Missing mount point argument for _get_block_size"
>> -		exit 1
>> +		_exit 1
>>   	fi
>>   	stat -f -c %S $1
>>   }
>> @@ -5146,14 +5143,14 @@ _run_hugepage_fsx() {
>>   	fi
>>   	cat $tmp.hugepage_fsx
>>   	rm -f $tmp.hugepage_fsx
>> -	test $res -ne 0 && exit 1
>> +	test $res -ne 0 && _exit 1
>>   	return 0
>>   }
>>   
>>   # run fsx or exit the test
>>   run_fsx()
>>   {
>> -	_run_fsx "$@" || exit 1
>> +	_run_fsx "$@" || _exit 1
>>   }
>>   
>>   _require_statx()
>> @@ -5318,7 +5315,7 @@ _get_max_file_size()
>>   {
>>   	if [ -z $1 ] || [ ! -d $1 ]; then
>>   		echo "Missing mount point argument for _get_max_file_size"
>> -		exit 1
>> +		_exit 1
>>   	fi
>>   
>>   	local mnt=$1
>> diff --git a/common/repair b/common/repair
>> index a79f9b2b..fd206f8e 100644
>> --- a/common/repair
>> +++ b/common/repair
>> @@ -16,7 +16,7 @@ _zero_position()
>>   		}'`
>>   	if [ -z "$offset" -o -z "$length" ]; then
>>   		echo "cannot calculate offset ($offset) or length ($length)"
>> -		exit
>> +		_exit 1
>>   	fi
>>   	length=`expr $length / 512`
>>   	$here/src/devzero -v $value -b 1 -n $length -o $offset $SCRATCH_DEV \
>> @@ -113,7 +113,7 @@ _filter_dd()
>>   }
>>   
>>   # do some controlled corrupting & ensure repair recovers us
>> -#
>> +#
>>   _check_repair()
>>   {
>>   	value=$1
>> diff --git a/common/xfs b/common/xfs
>> index 81d568d3..96c15f3c 100644
>> --- a/common/xfs
>> +++ b/common/xfs
>> @@ -553,7 +553,7 @@ _require_xfs_db_command()
>>   {
>>   	if [ $# -ne 1 ]; then
>>   		echo "Usage: _require_xfs_db_command command" 1>&2
>> -		exit 1
>> +		_exit 1
>>   	fi
>>   	command=$1
>>   
>> @@ -789,7 +789,7 @@ _check_xfs_filesystem()
>>   
>>   	if [ $# -ne 3 ]; then
>>   		echo "Usage: _check_xfs_filesystem device <logdev>|none <rtdev>|none" 1>&2
>> -		exit 1
>> +		_exit 1
>>   	fi
>>   
>>   	extra_mount_options=""
>> @@ -1014,7 +1014,7 @@ _check_xfs_filesystem()
>>   	if [ $ok -eq 0 ]; then
>>   		status=1
>>   		if [ "$iam" != "check" ]; then
>> -			exit 1
>> +			_exit 1
>>   		fi
>>   		return 1
>>   	fi
>> @@ -1379,7 +1379,7 @@ _require_xfs_spaceman_command()
>>   {
>>   	if [ -z "$1" ]; then
>>   		echo "Usage: _require_xfs_spaceman_command command [switch]" 1>&2
>> -		exit 1
>> +		_exit 1
>>   	fi
>>   	local command=$1
>>   	shift
>> -- 
>> 2.34.1
>>
>>
-- 
Nirjhar Roy
Linux Kernel Developer
IBM, Bangalore


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v4 6/6] common: exit --> _exit
  2025-04-09 15:24   ` Darrick J. Wong
  2025-04-09 16:29     ` Nirjhar Roy (IBM)
@ 2025-04-10 14:31     ` Zorro Lang
  2025-04-10 16:29       ` Darrick J. Wong
  1 sibling, 1 reply; 15+ messages in thread
From: Zorro Lang @ 2025-04-10 14:31 UTC (permalink / raw)
  To: Darrick J. Wong
  Cc: Nirjhar Roy (IBM), fstests, linux-ext4, linux-xfs, ritesh.list,
	ojaswin, zlang, david

On Wed, Apr 09, 2025 at 08:24:52AM -0700, Darrick J. Wong wrote:
> On Wed, Apr 09, 2025 at 07:00:52AM +0000, Nirjhar Roy (IBM) wrote:
> > Replace exit <return-val> with _exit <return-val> which
> > is introduced in the previous patch.
> > 
> > Signed-off-by: Nirjhar Roy (IBM) <nirjhar.roy.lists@gmail.com>
> > Reviewed-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>
> > Reviewed-by: Dave Chinner <dchinner@redhat.com>
> 
> Not sure why _filter_hole_fiemap gets whitespace cleanups in this patch,
> but I don't care to make Nirjhar rev the whole series just for
> whitespace so

Yeah, that doesn't make sense to this patch, but I don't mind merging that
anyway. If you hope to revert that part, I can help too :)

Thanks,
Zorro

> 
> Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
> 
> --D
> 
> > ---
> >  common/btrfs    |   6 +--
> >  common/ceph     |   2 +-
> >  common/config   |   7 ++--
> >  common/dump     |  11 +++--
> >  common/ext4     |   2 +-
> >  common/populate |   2 +-
> >  common/preamble |   2 +-
> >  common/punch    |  13 +++---
> >  common/rc       | 105 +++++++++++++++++++++++-------------------------
> >  common/repair   |   4 +-
> >  common/xfs      |   8 ++--
> >  11 files changed, 78 insertions(+), 84 deletions(-)
> > 
> > diff --git a/common/btrfs b/common/btrfs
> > index a3b9c12f..3725632c 100644
> > --- a/common/btrfs
> > +++ b/common/btrfs
> > @@ -80,7 +80,7 @@ _require_btrfs_mkfs_feature()
> >  {
> >  	if [ -z $1 ]; then
> >  		echo "Missing feature name argument for _require_btrfs_mkfs_feature"
> > -		exit 1
> > +		_exit 1
> >  	fi
> >  	feat=$1
> >  	$MKFS_BTRFS_PROG -O list-all 2>&1 | \
> > @@ -104,7 +104,7 @@ _require_btrfs_fs_feature()
> >  {
> >  	if [ -z $1 ]; then
> >  		echo "Missing feature name argument for _require_btrfs_fs_feature"
> > -		exit 1
> > +		_exit 1
> >  	fi
> >  	feat=$1
> >  	modprobe btrfs > /dev/null 2>&1
> > @@ -214,7 +214,7 @@ _check_btrfs_filesystem()
> >  	if [ $ok -eq 0 ]; then
> >  		status=1
> >  		if [ "$iam" != "check" ]; then
> > -			exit 1
> > +			_exit 1
> >  		fi
> >  		return 1
> >  	fi
> > diff --git a/common/ceph b/common/ceph
> > index d6f24df1..df7a6814 100644
> > --- a/common/ceph
> > +++ b/common/ceph
> > @@ -14,7 +14,7 @@ _ceph_create_file_layout()
> >  
> >  	if [ -e $fname ]; then
> >  		echo "File $fname already exists."
> > -		exit 1
> > +		_exit 1
> >  	fi
> >  	touch $fname
> >  	$SETFATTR_PROG -n ceph.file.layout \
> > diff --git a/common/config b/common/config
> > index 7dd78dbe..eada3971 100644
> > --- a/common/config
> > +++ b/common/config
> > @@ -124,8 +124,7 @@ set_mkfs_prog_path_with_opts()
> >  _fatal()
> >  {
> >      echo "$*"
> > -    status=1
> > -    exit 1
> > +    _exit 1
> >  }
> >  
> >  export MKFS_PROG="$(type -P mkfs)"
> > @@ -869,7 +868,7 @@ get_next_config() {
> >  		echo "Warning: need to define parameters for host $HOST"
> >  		echo "       or set variables:"
> >  		echo "       $MC"
> > -		exit 1
> > +		_exit 1
> >  	fi
> >  
> >  	_check_device TEST_DEV required $TEST_DEV
> > @@ -880,7 +879,7 @@ get_next_config() {
> >  	if [ ! -z "$SCRATCH_DEV_POOL" ]; then
> >  		if [ ! -z "$SCRATCH_DEV" ]; then
> >  			echo "common/config: Error: \$SCRATCH_DEV ($SCRATCH_DEV) should be unset when \$SCRATCH_DEV_POOL ($SCRATCH_DEV_POOL) is set"
> > -			exit 1
> > +			_exit 1
> >  		fi
> >  		SCRATCH_DEV=`echo $SCRATCH_DEV_POOL | awk '{print $1}'`
> >  		export SCRATCH_DEV
> > diff --git a/common/dump b/common/dump
> > index 6dcd6250..09859006 100644
> > --- a/common/dump
> > +++ b/common/dump
> > @@ -280,8 +280,7 @@ _create_dumpdir_stress_num()
> >      rm -rf $dump_dir
> >      if ! mkdir $dump_dir; then
> >          echo "    failed to mkdir $dump_dir"
> > -        status=1
> > -        exit
> > +        _exit 1
> >      fi
> >  
> >      # Remove fsstress commands that aren't supported on all xfs configs so that
> > @@ -480,7 +479,7 @@ _do_create_dumpdir_fill()
> >  		else
> >  		    $verbose && echo
> >  		    echo "Error: cannot mkdir \"$dir\""
> > -		    exit 1
> > +		    _exit 1
> >  		fi
> >  	    fi
> >  	else
> > @@ -496,7 +495,7 @@ _do_create_dumpdir_fill()
> >  		    else
> >  			$verbose && echo
> >  			echo "Error: cannot mkdir \"$dir\""
> > -			exit 1
> > +			_exit 1
> >  		    fi
> >  		fi
> >  	    fi
> > @@ -507,7 +506,7 @@ _do_create_dumpdir_fill()
> >  	    else
> >  		$verbose && echo
> >  		echo "Error: cannot create \"$file\""
> > -		exit 1
> > +		_exit 1
> >  	    fi
> >  	fi
> >  	if [ -n "$owner" -a -n "$group" ]; then
> > @@ -649,7 +648,7 @@ _do_create_dump_symlinks()
> >  		else
> >  		    $verbose && echo
> >  		    echo "Error: cannot mkdir \"$dir\""
> > -		    exit 1
> > +		    _exit 1
> >  		fi
> >  	    fi
> >  	fi
> > diff --git a/common/ext4 b/common/ext4
> > index e1b336d3..f88fa532 100644
> > --- a/common/ext4
> > +++ b/common/ext4
> > @@ -182,7 +182,7 @@ _require_scratch_ext4_feature()
> >  {
> >      if [ -z "$1" ]; then
> >          echo "Usage: _require_scratch_ext4_feature feature"
> > -        exit 1
> > +        _exit 1
> >      fi
> >      $MKFS_EXT4_PROG -F $MKFS_OPTIONS -O "$1" \
> >  		    $SCRATCH_DEV 512m >/dev/null 2>&1 \
> > diff --git a/common/populate b/common/populate
> > index 7352f598..50dc75d3 100644
> > --- a/common/populate
> > +++ b/common/populate
> > @@ -1003,7 +1003,7 @@ _fill_fs()
> >  
> >  	if [ $# -ne 4 ]; then
> >  		echo "Usage: _fill_fs filesize dir blocksize switch_user"
> > -		exit 1
> > +		_exit 1
> >  	fi
> >  
> >  	if [ $switch_user -eq 0 ]; then
> > diff --git a/common/preamble b/common/preamble
> > index c92e55bb..ba029a34 100644
> > --- a/common/preamble
> > +++ b/common/preamble
> > @@ -35,7 +35,7 @@ _begin_fstest()
> >  {
> >  	if [ -n "$seq" ]; then
> >  		echo "_begin_fstest can only be called once!"
> > -		exit 1
> > +		_exit 1
> >  	fi
> >  
> >  	seq=`basename $0`
> > diff --git a/common/punch b/common/punch
> > index 43ccab69..64d665d8 100644
> > --- a/common/punch
> > +++ b/common/punch
> > @@ -172,16 +172,16 @@ _filter_fiemap_flags()
> >  	$AWK_PROG -e "$awk_script" | _coalesce_extents
> >  }
> >  
> > -# Filters fiemap output to only print the 
> > +# Filters fiemap output to only print the
> >  # file offset column and whether or not
> >  # it is an extent or a hole
> >  _filter_hole_fiemap()
> >  {
> >  	$AWK_PROG '
> >  		$3 ~ /hole/ {
> > -			print $1, $2, $3; 
> > +			print $1, $2, $3;
> >  			next;
> > -		}   
> > +		}
> >  		$5 ~ /0x[[:xdigit:]]+/ {
> >  			print $1, $2, "extent";
> >  		}' |
> > @@ -224,8 +224,7 @@ _filter_bmap()
> >  
> >  die_now()
> >  {
> > -	status=1
> > -	exit
> > +	_exit 1
> >  }
> >  
> >  # test the different corner cases for zeroing a range:
> > @@ -276,7 +275,7 @@ _test_generic_punch()
> >  		u)	unwritten_tests=
> >  		;;
> >  		?)	echo Invalid flag
> > -		exit 1
> > +		_exit 1
> >  		;;
> >  		esac
> >  	done
> > @@ -552,7 +551,7 @@ _test_block_boundaries()
> >  		d)	sync_cmd=
> >  		;;
> >  		?)	echo Invalid flag
> > -		exit 1
> > +		_exit 1
> >  		;;
> >  		esac
> >  	done
> > diff --git a/common/rc b/common/rc
> > index 038c22f6..3b21eb27 100644
> > --- a/common/rc
> > +++ b/common/rc
> > @@ -909,8 +909,7 @@ _mkfs_dev()
> >  	# output stored mkfs output
> >  	cat $tmp.mkfserr >&2
> >  	cat $tmp.mkfsstd
> > -	status=1
> > -	exit 1
> > +	_exit 1
> >      fi
> >      rm -f $tmp.mkfserr $tmp.mkfsstd
> >  }
> > @@ -1575,7 +1574,7 @@ _get_pids_by_name()
> >      if [ $# -ne 1 ]
> >      then
> >  	echo "Usage: _get_pids_by_name process-name" 1>&2
> > -	exit 1
> > +	_exit 1
> >      fi
> >  
> >      # Algorithm ... all ps(1) variants have a time of the form MM:SS or
> > @@ -1609,7 +1608,7 @@ _df_device()
> >      if [ $# -ne 1 ]
> >      then
> >  	echo "Usage: _df_device device" 1>&2
> > -	exit 1
> > +	_exit 1
> >      fi
> >  
> >      # Note that we use "==" here so awk doesn't try to interpret an NFS over
> > @@ -1641,7 +1640,7 @@ _df_dir()
> >      if [ $# -ne 1 ]
> >      then
> >  	echo "Usage: _df_dir device" 1>&2
> > -	exit 1
> > +	_exit 1
> >      fi
> >  
> >      $DF_PROG $1 2>/dev/null | $AWK_PROG -v what=$1 '
> > @@ -1667,7 +1666,7 @@ _used()
> >      if [ $# -ne 1 ]
> >      then
> >  	echo "Usage: _used device" 1>&2
> > -	exit 1
> > +	_exit 1
> >      fi
> >  
> >      _df_device $1 | $AWK_PROG '{ sub("%", "") ; print $6 }'
> > @@ -1680,7 +1679,7 @@ _fs_type()
> >      if [ $# -ne 1 ]
> >      then
> >  	echo "Usage: _fs_type device" 1>&2
> > -	exit 1
> > +	_exit 1
> >      fi
> >  
> >      #
> > @@ -1705,7 +1704,7 @@ _fs_options()
> >      if [ $# -ne 1 ]
> >      then
> >  	echo "Usage: _fs_options device" 1>&2
> > -	exit 1
> > +	_exit 1
> >      fi
> >  
> >      $AWK_PROG -v dev=$1 '
> > @@ -1720,7 +1719,7 @@ _is_block_dev()
> >      if [ $# -ne 1 ]
> >      then
> >  	echo "Usage: _is_block_dev dev" 1>&2
> > -	exit 1
> > +	_exit 1
> >      fi
> >  
> >      local dev=$1
> > @@ -1739,7 +1738,7 @@ _is_char_dev()
> >  {
> >  	if [ $# -ne 1 ]; then
> >  		echo "Usage: _is_char_dev dev" 1>&2
> > -		exit 1
> > +		_exit 1
> >  	fi
> >  
> >  	local dev=$1
> > @@ -1772,7 +1771,7 @@ _do()
> >  	echo -n "$note... "
> >      else
> >  	echo "Usage: _do [note] cmd" 1>&2
> > -	status=1; exit
> > +	_exit 1
> >      fi
> >  
> >      (eval "echo '---' \"$cmd\"") >>$seqres.full
> > @@ -1793,7 +1792,7 @@ _do()
> >      then
> >  	[ $# -ne 2 ] && echo
> >  	eval "echo \"$cmd\" failed \(returned $ret\): see $seqres.full"
> > -	status=1; exit
> > +	_exit 1
> >      fi
> >  
> >      return $ret
> > @@ -1809,8 +1808,7 @@ _notrun()
> >      rm -f ${RESULT_DIR}/require_test*
> >      rm -f ${RESULT_DIR}/require_scratch*
> >  
> > -    status=0
> > -    exit
> > +    _exit 0
> >  }
> >  
> >  # just plain bail out
> > @@ -1819,8 +1817,7 @@ _fail()
> >  {
> >      echo "$*" | tee -a $seqres.full
> >      echo "(see $seqres.full for details)"
> > -    status=1
> > -    exit 1
> > +    _exit 1
> >  }
> >  
> >  #
> > @@ -2049,14 +2046,14 @@ _require_scratch_nocheck()
> >  
> >      _check_mounted_on SCRATCH_DEV $SCRATCH_DEV SCRATCH_MNT $SCRATCH_MNT
> >      local err=$?
> > -    [ $err -le 1 ] || exit 1
> > +    [ $err -le 1 ] || _exit 1
> >      if [ $err -eq 0 ]
> >      then
> >          # if it's mounted, unmount it
> >          if ! _scratch_unmount
> >          then
> >              echo "failed to unmount $SCRATCH_DEV"
> > -            exit 1
> > +            _exit 1
> >          fi
> >      fi
> >      rm -f ${RESULT_DIR}/require_scratch "$RESULT_DIR/.skip_orebuild" "$RESULT_DIR/.skip_rebuild"
> > @@ -2273,13 +2270,13 @@ _require_test()
> >  
> >      _check_mounted_on TEST_DEV $TEST_DEV TEST_DIR $TEST_DIR
> >      local err=$?
> > -    [ $err -le 1 ] || exit 1
> > +    [ $err -le 1 ] || _exit 1
> >      if [ $err -ne 0 ]
> >      then
> >  	if ! _test_mount
> >  	then
> >  		echo "!!! failed to mount $TEST_DEV on $TEST_DIR"
> > -		exit 1
> > +		_exit 1
> >  	fi
> >      fi
> >      touch ${RESULT_DIR}/require_test
> > @@ -2391,7 +2388,7 @@ _require_block_device()
> >  {
> >  	if [ -z "$1" ]; then
> >  		echo "Usage: _require_block_device <dev>" 1>&2
> > -		exit 1
> > +		_exit 1
> >  	fi
> >  	if [ "`_is_block_dev "$1"`" == "" ]; then
> >  		_notrun "require $1 to be valid block disk"
> > @@ -2404,7 +2401,7 @@ _require_local_device()
> >  {
> >  	if [ -z "$1" ]; then
> >  		echo "Usage: _require_local_device <dev>" 1>&2
> > -		exit 1
> > +		_exit 1
> >  	fi
> >  	if [ "`_is_block_dev "$1"`" != "" ]; then
> >  		return 0
> > @@ -2512,7 +2509,7 @@ _zone_type()
> >  	local target=$1
> >  	if [ -z $target ]; then
> >  		echo "Usage: _zone_type <device>"
> > -		exit 1
> > +		_exit 1
> >  	fi
> >  	local sdev=`_short_dev $target`
> >  
> > @@ -2528,7 +2525,7 @@ _require_zoned_device()
> >  	local target=$1
> >  	if [ -z $target ]; then
> >  		echo "Usage: _require_zoned_device <device>"
> > -		exit 1
> > +		_exit 1
> >  	fi
> >  
> >  	local type=`_zone_type ${target}`
> > @@ -2668,7 +2665,7 @@ _run_aiodio()
> >      if [ -z "$1" ]
> >      then
> >          echo "usage: _run_aiodio command_name" 2>&1
> > -        status=1; exit 1
> > +        _exit 1
> >      fi
> >  
> >      _require_aiodio $1
> > @@ -2880,7 +2877,7 @@ _require_xfs_io_command()
> >  	if [ -z "$1" ]
> >  	then
> >  		echo "Usage: _require_xfs_io_command command [switch]" 1>&2
> > -		exit 1
> > +		_exit 1
> >  	fi
> >  	local command=$1
> >  	shift
> > @@ -3364,7 +3361,7 @@ _is_dev_mounted()
> >  
> >  	if [ $# -lt 1 ]; then
> >  		echo "Usage: _is_dev_mounted <device> [fstype]" 1>&2
> > -		exit 1
> > +		_exit 1
> >  	fi
> >  
> >  	findmnt -rncv -S $dev -t $fstype -o TARGET | head -1
> > @@ -3378,7 +3375,7 @@ _is_dir_mountpoint()
> >  
> >  	if [ $# -lt 1 ]; then
> >  		echo "Uasge: _is_dir_mountpoint <dir> [fstype]" 1>&2
> > -		exit 1
> > +		_exit 1
> >  	fi
> >  
> >  	findmnt -rncv -t $fstype -o TARGET $dir | head -1
> > @@ -3391,7 +3388,7 @@ _remount()
> >      if [ $# -ne 2 ]
> >      then
> >  	echo "Usage: _remount device ro/rw" 1>&2
> > -	exit 1
> > +	_exit 1
> >      fi
> >      local device=$1
> >      local mode=$2
> > @@ -3399,7 +3396,7 @@ _remount()
> >      if ! mount -o remount,$mode $device
> >      then
> >          echo "_remount: failed to remount filesystem on $device as $mode"
> > -        exit 1
> > +        _exit 1
> >      fi
> >  }
> >  
> > @@ -3417,7 +3414,7 @@ _umount_or_remount_ro()
> >      if [ $# -ne 1 ]
> >      then
> >  	echo "Usage: _umount_or_remount_ro <device>" 1>&2
> > -	exit 1
> > +	_exit 1
> >      fi
> >  
> >      local device=$1
> > @@ -3435,7 +3432,7 @@ _mount_or_remount_rw()
> >  {
> >  	if [ $# -ne 3 ]; then
> >  		echo "Usage: _mount_or_remount_rw <opts> <dev> <mnt>" 1>&2
> > -		exit 1
> > +		_exit 1
> >  	fi
> >  	local mount_opts=$1
> >  	local device=$2
> > @@ -3516,7 +3513,7 @@ _check_generic_filesystem()
> >      if [ $ok -eq 0 ]; then
> >  	status=1
> >  	if [ "$iam" != "check" ]; then
> > -		exit 1
> > +		_exit 1
> >  	fi
> >  	return 1
> >      fi
> > @@ -3582,7 +3579,7 @@ _check_udf_filesystem()
> >      if [ $# -ne 1 -a $# -ne 2 ]
> >      then
> >  	echo "Usage: _check_udf_filesystem device [last_block]" 1>&2
> > -	exit 1
> > +	_exit 1
> >      fi
> >  
> >      if [ ! -x $here/src/udf_test ]
> > @@ -3776,7 +3773,7 @@ _get_os_name()
> >  		echo 'linux'
> >  	else
> >  		echo Unknown operating system: `uname`
> > -		exit
> > +		_exit 1
> >  	fi
> >  }
> >  
> > @@ -3837,7 +3834,7 @@ _link_out_file()
> >  _die()
> >  {
> >          echo $@
> > -        exit 1
> > +        _exit 1
> >  }
> >  
> >  # convert urandom incompressible data to compressible text data
> > @@ -3994,7 +3991,7 @@ _require_scratch_dev_pool()
> >  		if _mount | grep -q $i; then
> >  			if ! _unmount $i; then
> >  		            echo "failed to unmount $i - aborting"
> > -		            exit 1
> > +		            _exit 1
> >  		        fi
> >  		fi
> >  		# To help better debug when something fails, we remove
> > @@ -4403,7 +4400,7 @@ _require_batched_discard()
> >  {
> >  	if [ $# -ne 1 ]; then
> >  		echo "Usage: _require_batched_discard mnt_point" 1>&2
> > -		exit 1
> > +		_exit 1
> >  	fi
> >  	_require_fstrim
> >  
> > @@ -4630,7 +4627,7 @@ _require_chattr()
> >  {
> >  	if [ -z "$1" ]; then
> >  		echo "Usage: _require_chattr <attr>"
> > -		exit 1
> > +		_exit 1
> >  	fi
> >  	local attribute=$1
> >  
> > @@ -4649,7 +4646,7 @@ _get_total_inode()
> >  {
> >  	if [ -z "$1" ]; then
> >  		echo "Usage: _get_total_inode <mnt>"
> > -		exit 1
> > +		_exit 1
> >  	fi
> >  	local nr_inode;
> >  	nr_inode=`$DF_PROG -i $1 | tail -1 | awk '{print $3}'`
> > @@ -4660,7 +4657,7 @@ _get_used_inode()
> >  {
> >  	if [ -z "$1" ]; then
> >  		echo "Usage: _get_used_inode <mnt>"
> > -		exit 1
> > +		_exit 1
> >  	fi
> >  	local nr_inode;
> >  	nr_inode=`$DF_PROG -i $1 | tail -1 | awk '{print $4}'`
> > @@ -4671,7 +4668,7 @@ _get_used_inode_percent()
> >  {
> >  	if [ -z "$1" ]; then
> >  		echo "Usage: _get_used_inode_percent <mnt>"
> > -		exit 1
> > +		_exit 1
> >  	fi
> >  	local pct_inode;
> >  	pct_inode=`$DF_PROG -i $1 | tail -1 | awk '{ print $6 }' | \
> > @@ -4683,7 +4680,7 @@ _get_free_inode()
> >  {
> >  	if [ -z "$1" ]; then
> >  		echo "Usage: _get_free_inode <mnt>"
> > -		exit 1
> > +		_exit 1
> >  	fi
> >  	local nr_inode;
> >  	nr_inode=`$DF_PROG -i $1 | tail -1 | awk '{print $5}'`
> > @@ -4696,7 +4693,7 @@ _get_available_space()
> >  {
> >  	if [ -z "$1" ]; then
> >  		echo "Usage: _get_available_space <mnt>"
> > -		exit 1
> > +		_exit 1
> >  	fi
> >  	$DF_PROG -B 1 $1 | tail -n1 | awk '{ print $5 }'
> >  }
> > @@ -4707,7 +4704,7 @@ _get_total_space()
> >  {
> >  	if [ -z "$1" ]; then
> >  		echo "Usage: _get_total_space <mnt>"
> > -		exit 1
> > +		_exit 1
> >  	fi
> >  	$DF_PROG -B 1 $1 | tail -n1 | awk '{ print $3 }'
> >  }
> > @@ -4952,7 +4949,7 @@ init_rc()
> >  	if [ "$TEST_DEV" = ""  ]
> >  	then
> >  		echo "common/rc: Error: \$TEST_DEV is not set"
> > -		exit 1
> > +		_exit 1
> >  	fi
> >  
> >  	# if $TEST_DEV is not mounted, mount it now as XFS
> > @@ -4966,20 +4963,20 @@ init_rc()
> >  			if ! _test_mount
> >  			then
> >  				echo "common/rc: could not mount $TEST_DEV on $TEST_DIR"
> > -				exit 1
> > +				_exit 1
> >  			fi
> >  		fi
> >  	fi
> >  
> >  	# Sanity check that TEST partition is not mounted at another mount point
> >  	# or as another fs type
> > -	_check_mounted_on TEST_DEV $TEST_DEV TEST_DIR $TEST_DIR $FSTYP || exit 1
> > +	_check_mounted_on TEST_DEV $TEST_DEV TEST_DIR $TEST_DIR $FSTYP || _exit 1
> >  	if [ -n "$SCRATCH_DEV" ]; then
> >  		# Sanity check that SCRATCH partition is not mounted at another
> >  		# mount point, because it is about to be unmounted and formatted.
> >  		# Another fs type for scratch is fine (bye bye old fs type).
> >  		_check_mounted_on SCRATCH_DEV $SCRATCH_DEV SCRATCH_MNT $SCRATCH_MNT
> > -		[ $? -le 1 ] || exit 1
> > +		[ $? -le 1 ] || _exit 1
> >  	fi
> >  
> >  	# Figure out if we need to add -F ("foreign", deprecated) option to xfs_io
> > @@ -5029,7 +5026,7 @@ _get_file_block_size()
> >  {
> >  	if [ -z $1 ] || [ ! -d $1 ]; then
> >  		echo "Missing mount point argument for _get_file_block_size"
> > -		exit 1
> > +		_exit 1
> >  	fi
> >  
> >  	case "$FSTYP" in
> > @@ -5076,7 +5073,7 @@ _get_block_size()
> >  {
> >  	if [ -z $1 ] || [ ! -d $1 ]; then
> >  		echo "Missing mount point argument for _get_block_size"
> > -		exit 1
> > +		_exit 1
> >  	fi
> >  	stat -f -c %S $1
> >  }
> > @@ -5146,14 +5143,14 @@ _run_hugepage_fsx() {
> >  	fi
> >  	cat $tmp.hugepage_fsx
> >  	rm -f $tmp.hugepage_fsx
> > -	test $res -ne 0 && exit 1
> > +	test $res -ne 0 && _exit 1
> >  	return 0
> >  }
> >  
> >  # run fsx or exit the test
> >  run_fsx()
> >  {
> > -	_run_fsx "$@" || exit 1
> > +	_run_fsx "$@" || _exit 1
> >  }
> >  
> >  _require_statx()
> > @@ -5318,7 +5315,7 @@ _get_max_file_size()
> >  {
> >  	if [ -z $1 ] || [ ! -d $1 ]; then
> >  		echo "Missing mount point argument for _get_max_file_size"
> > -		exit 1
> > +		_exit 1
> >  	fi
> >  
> >  	local mnt=$1
> > diff --git a/common/repair b/common/repair
> > index a79f9b2b..fd206f8e 100644
> > --- a/common/repair
> > +++ b/common/repair
> > @@ -16,7 +16,7 @@ _zero_position()
> >  		}'`
> >  	if [ -z "$offset" -o -z "$length" ]; then
> >  		echo "cannot calculate offset ($offset) or length ($length)"
> > -		exit
> > +		_exit 1
> >  	fi
> >  	length=`expr $length / 512`
> >  	$here/src/devzero -v $value -b 1 -n $length -o $offset $SCRATCH_DEV \
> > @@ -113,7 +113,7 @@ _filter_dd()
> >  }
> >  
> >  # do some controlled corrupting & ensure repair recovers us
> > -# 
> > +#
> >  _check_repair()
> >  {
> >  	value=$1
> > diff --git a/common/xfs b/common/xfs
> > index 81d568d3..96c15f3c 100644
> > --- a/common/xfs
> > +++ b/common/xfs
> > @@ -553,7 +553,7 @@ _require_xfs_db_command()
> >  {
> >  	if [ $# -ne 1 ]; then
> >  		echo "Usage: _require_xfs_db_command command" 1>&2
> > -		exit 1
> > +		_exit 1
> >  	fi
> >  	command=$1
> >  
> > @@ -789,7 +789,7 @@ _check_xfs_filesystem()
> >  
> >  	if [ $# -ne 3 ]; then
> >  		echo "Usage: _check_xfs_filesystem device <logdev>|none <rtdev>|none" 1>&2
> > -		exit 1
> > +		_exit 1
> >  	fi
> >  
> >  	extra_mount_options=""
> > @@ -1014,7 +1014,7 @@ _check_xfs_filesystem()
> >  	if [ $ok -eq 0 ]; then
> >  		status=1
> >  		if [ "$iam" != "check" ]; then
> > -			exit 1
> > +			_exit 1
> >  		fi
> >  		return 1
> >  	fi
> > @@ -1379,7 +1379,7 @@ _require_xfs_spaceman_command()
> >  {
> >  	if [ -z "$1" ]; then
> >  		echo "Usage: _require_xfs_spaceman_command command [switch]" 1>&2
> > -		exit 1
> > +		_exit 1
> >  	fi
> >  	local command=$1
> >  	shift
> > -- 
> > 2.34.1
> > 
> > 
> 


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v4 6/6] common: exit --> _exit
  2025-04-10 14:31     ` Zorro Lang
@ 2025-04-10 16:29       ` Darrick J. Wong
  0 siblings, 0 replies; 15+ messages in thread
From: Darrick J. Wong @ 2025-04-10 16:29 UTC (permalink / raw)
  To: Zorro Lang
  Cc: Nirjhar Roy (IBM), fstests, linux-ext4, linux-xfs, ritesh.list,
	ojaswin, zlang, david

On Thu, Apr 10, 2025 at 10:31:57PM +0800, Zorro Lang wrote:
> On Wed, Apr 09, 2025 at 08:24:52AM -0700, Darrick J. Wong wrote:
> > On Wed, Apr 09, 2025 at 07:00:52AM +0000, Nirjhar Roy (IBM) wrote:
> > > Replace exit <return-val> with _exit <return-val> which
> > > is introduced in the previous patch.
> > > 
> > > Signed-off-by: Nirjhar Roy (IBM) <nirjhar.roy.lists@gmail.com>
> > > Reviewed-by: Ritesh Harjani (IBM) <ritesh.list@gmail.com>
> > > Reviewed-by: Dave Chinner <dchinner@redhat.com>
> > 
> > Not sure why _filter_hole_fiemap gets whitespace cleanups in this patch,
> > but I don't care to make Nirjhar rev the whole series just for
> > whitespace so
> 
> Yeah, that doesn't make sense to this patch, but I don't mind merging that
> anyway. If you hope to revert that part, I can help too :)

Nah, just let it go in, it's not a major change.

--D

> Thanks,
> Zorro
> 
> > 
> > Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>
> > 
> > --D
> > 
> > > ---
> > >  common/btrfs    |   6 +--
> > >  common/ceph     |   2 +-
> > >  common/config   |   7 ++--
> > >  common/dump     |  11 +++--
> > >  common/ext4     |   2 +-
> > >  common/populate |   2 +-
> > >  common/preamble |   2 +-
> > >  common/punch    |  13 +++---
> > >  common/rc       | 105 +++++++++++++++++++++++-------------------------
> > >  common/repair   |   4 +-
> > >  common/xfs      |   8 ++--
> > >  11 files changed, 78 insertions(+), 84 deletions(-)
> > > 
> > > diff --git a/common/btrfs b/common/btrfs
> > > index a3b9c12f..3725632c 100644
> > > --- a/common/btrfs
> > > +++ b/common/btrfs
> > > @@ -80,7 +80,7 @@ _require_btrfs_mkfs_feature()
> > >  {
> > >  	if [ -z $1 ]; then
> > >  		echo "Missing feature name argument for _require_btrfs_mkfs_feature"
> > > -		exit 1
> > > +		_exit 1
> > >  	fi
> > >  	feat=$1
> > >  	$MKFS_BTRFS_PROG -O list-all 2>&1 | \
> > > @@ -104,7 +104,7 @@ _require_btrfs_fs_feature()
> > >  {
> > >  	if [ -z $1 ]; then
> > >  		echo "Missing feature name argument for _require_btrfs_fs_feature"
> > > -		exit 1
> > > +		_exit 1
> > >  	fi
> > >  	feat=$1
> > >  	modprobe btrfs > /dev/null 2>&1
> > > @@ -214,7 +214,7 @@ _check_btrfs_filesystem()
> > >  	if [ $ok -eq 0 ]; then
> > >  		status=1
> > >  		if [ "$iam" != "check" ]; then
> > > -			exit 1
> > > +			_exit 1
> > >  		fi
> > >  		return 1
> > >  	fi
> > > diff --git a/common/ceph b/common/ceph
> > > index d6f24df1..df7a6814 100644
> > > --- a/common/ceph
> > > +++ b/common/ceph
> > > @@ -14,7 +14,7 @@ _ceph_create_file_layout()
> > >  
> > >  	if [ -e $fname ]; then
> > >  		echo "File $fname already exists."
> > > -		exit 1
> > > +		_exit 1
> > >  	fi
> > >  	touch $fname
> > >  	$SETFATTR_PROG -n ceph.file.layout \
> > > diff --git a/common/config b/common/config
> > > index 7dd78dbe..eada3971 100644
> > > --- a/common/config
> > > +++ b/common/config
> > > @@ -124,8 +124,7 @@ set_mkfs_prog_path_with_opts()
> > >  _fatal()
> > >  {
> > >      echo "$*"
> > > -    status=1
> > > -    exit 1
> > > +    _exit 1
> > >  }
> > >  
> > >  export MKFS_PROG="$(type -P mkfs)"
> > > @@ -869,7 +868,7 @@ get_next_config() {
> > >  		echo "Warning: need to define parameters for host $HOST"
> > >  		echo "       or set variables:"
> > >  		echo "       $MC"
> > > -		exit 1
> > > +		_exit 1
> > >  	fi
> > >  
> > >  	_check_device TEST_DEV required $TEST_DEV
> > > @@ -880,7 +879,7 @@ get_next_config() {
> > >  	if [ ! -z "$SCRATCH_DEV_POOL" ]; then
> > >  		if [ ! -z "$SCRATCH_DEV" ]; then
> > >  			echo "common/config: Error: \$SCRATCH_DEV ($SCRATCH_DEV) should be unset when \$SCRATCH_DEV_POOL ($SCRATCH_DEV_POOL) is set"
> > > -			exit 1
> > > +			_exit 1
> > >  		fi
> > >  		SCRATCH_DEV=`echo $SCRATCH_DEV_POOL | awk '{print $1}'`
> > >  		export SCRATCH_DEV
> > > diff --git a/common/dump b/common/dump
> > > index 6dcd6250..09859006 100644
> > > --- a/common/dump
> > > +++ b/common/dump
> > > @@ -280,8 +280,7 @@ _create_dumpdir_stress_num()
> > >      rm -rf $dump_dir
> > >      if ! mkdir $dump_dir; then
> > >          echo "    failed to mkdir $dump_dir"
> > > -        status=1
> > > -        exit
> > > +        _exit 1
> > >      fi
> > >  
> > >      # Remove fsstress commands that aren't supported on all xfs configs so that
> > > @@ -480,7 +479,7 @@ _do_create_dumpdir_fill()
> > >  		else
> > >  		    $verbose && echo
> > >  		    echo "Error: cannot mkdir \"$dir\""
> > > -		    exit 1
> > > +		    _exit 1
> > >  		fi
> > >  	    fi
> > >  	else
> > > @@ -496,7 +495,7 @@ _do_create_dumpdir_fill()
> > >  		    else
> > >  			$verbose && echo
> > >  			echo "Error: cannot mkdir \"$dir\""
> > > -			exit 1
> > > +			_exit 1
> > >  		    fi
> > >  		fi
> > >  	    fi
> > > @@ -507,7 +506,7 @@ _do_create_dumpdir_fill()
> > >  	    else
> > >  		$verbose && echo
> > >  		echo "Error: cannot create \"$file\""
> > > -		exit 1
> > > +		_exit 1
> > >  	    fi
> > >  	fi
> > >  	if [ -n "$owner" -a -n "$group" ]; then
> > > @@ -649,7 +648,7 @@ _do_create_dump_symlinks()
> > >  		else
> > >  		    $verbose && echo
> > >  		    echo "Error: cannot mkdir \"$dir\""
> > > -		    exit 1
> > > +		    _exit 1
> > >  		fi
> > >  	    fi
> > >  	fi
> > > diff --git a/common/ext4 b/common/ext4
> > > index e1b336d3..f88fa532 100644
> > > --- a/common/ext4
> > > +++ b/common/ext4
> > > @@ -182,7 +182,7 @@ _require_scratch_ext4_feature()
> > >  {
> > >      if [ -z "$1" ]; then
> > >          echo "Usage: _require_scratch_ext4_feature feature"
> > > -        exit 1
> > > +        _exit 1
> > >      fi
> > >      $MKFS_EXT4_PROG -F $MKFS_OPTIONS -O "$1" \
> > >  		    $SCRATCH_DEV 512m >/dev/null 2>&1 \
> > > diff --git a/common/populate b/common/populate
> > > index 7352f598..50dc75d3 100644
> > > --- a/common/populate
> > > +++ b/common/populate
> > > @@ -1003,7 +1003,7 @@ _fill_fs()
> > >  
> > >  	if [ $# -ne 4 ]; then
> > >  		echo "Usage: _fill_fs filesize dir blocksize switch_user"
> > > -		exit 1
> > > +		_exit 1
> > >  	fi
> > >  
> > >  	if [ $switch_user -eq 0 ]; then
> > > diff --git a/common/preamble b/common/preamble
> > > index c92e55bb..ba029a34 100644
> > > --- a/common/preamble
> > > +++ b/common/preamble
> > > @@ -35,7 +35,7 @@ _begin_fstest()
> > >  {
> > >  	if [ -n "$seq" ]; then
> > >  		echo "_begin_fstest can only be called once!"
> > > -		exit 1
> > > +		_exit 1
> > >  	fi
> > >  
> > >  	seq=`basename $0`
> > > diff --git a/common/punch b/common/punch
> > > index 43ccab69..64d665d8 100644
> > > --- a/common/punch
> > > +++ b/common/punch
> > > @@ -172,16 +172,16 @@ _filter_fiemap_flags()
> > >  	$AWK_PROG -e "$awk_script" | _coalesce_extents
> > >  }
> > >  
> > > -# Filters fiemap output to only print the 
> > > +# Filters fiemap output to only print the
> > >  # file offset column and whether or not
> > >  # it is an extent or a hole
> > >  _filter_hole_fiemap()
> > >  {
> > >  	$AWK_PROG '
> > >  		$3 ~ /hole/ {
> > > -			print $1, $2, $3; 
> > > +			print $1, $2, $3;
> > >  			next;
> > > -		}   
> > > +		}
> > >  		$5 ~ /0x[[:xdigit:]]+/ {
> > >  			print $1, $2, "extent";
> > >  		}' |
> > > @@ -224,8 +224,7 @@ _filter_bmap()
> > >  
> > >  die_now()
> > >  {
> > > -	status=1
> > > -	exit
> > > +	_exit 1
> > >  }
> > >  
> > >  # test the different corner cases for zeroing a range:
> > > @@ -276,7 +275,7 @@ _test_generic_punch()
> > >  		u)	unwritten_tests=
> > >  		;;
> > >  		?)	echo Invalid flag
> > > -		exit 1
> > > +		_exit 1
> > >  		;;
> > >  		esac
> > >  	done
> > > @@ -552,7 +551,7 @@ _test_block_boundaries()
> > >  		d)	sync_cmd=
> > >  		;;
> > >  		?)	echo Invalid flag
> > > -		exit 1
> > > +		_exit 1
> > >  		;;
> > >  		esac
> > >  	done
> > > diff --git a/common/rc b/common/rc
> > > index 038c22f6..3b21eb27 100644
> > > --- a/common/rc
> > > +++ b/common/rc
> > > @@ -909,8 +909,7 @@ _mkfs_dev()
> > >  	# output stored mkfs output
> > >  	cat $tmp.mkfserr >&2
> > >  	cat $tmp.mkfsstd
> > > -	status=1
> > > -	exit 1
> > > +	_exit 1
> > >      fi
> > >      rm -f $tmp.mkfserr $tmp.mkfsstd
> > >  }
> > > @@ -1575,7 +1574,7 @@ _get_pids_by_name()
> > >      if [ $# -ne 1 ]
> > >      then
> > >  	echo "Usage: _get_pids_by_name process-name" 1>&2
> > > -	exit 1
> > > +	_exit 1
> > >      fi
> > >  
> > >      # Algorithm ... all ps(1) variants have a time of the form MM:SS or
> > > @@ -1609,7 +1608,7 @@ _df_device()
> > >      if [ $# -ne 1 ]
> > >      then
> > >  	echo "Usage: _df_device device" 1>&2
> > > -	exit 1
> > > +	_exit 1
> > >      fi
> > >  
> > >      # Note that we use "==" here so awk doesn't try to interpret an NFS over
> > > @@ -1641,7 +1640,7 @@ _df_dir()
> > >      if [ $# -ne 1 ]
> > >      then
> > >  	echo "Usage: _df_dir device" 1>&2
> > > -	exit 1
> > > +	_exit 1
> > >      fi
> > >  
> > >      $DF_PROG $1 2>/dev/null | $AWK_PROG -v what=$1 '
> > > @@ -1667,7 +1666,7 @@ _used()
> > >      if [ $# -ne 1 ]
> > >      then
> > >  	echo "Usage: _used device" 1>&2
> > > -	exit 1
> > > +	_exit 1
> > >      fi
> > >  
> > >      _df_device $1 | $AWK_PROG '{ sub("%", "") ; print $6 }'
> > > @@ -1680,7 +1679,7 @@ _fs_type()
> > >      if [ $# -ne 1 ]
> > >      then
> > >  	echo "Usage: _fs_type device" 1>&2
> > > -	exit 1
> > > +	_exit 1
> > >      fi
> > >  
> > >      #
> > > @@ -1705,7 +1704,7 @@ _fs_options()
> > >      if [ $# -ne 1 ]
> > >      then
> > >  	echo "Usage: _fs_options device" 1>&2
> > > -	exit 1
> > > +	_exit 1
> > >      fi
> > >  
> > >      $AWK_PROG -v dev=$1 '
> > > @@ -1720,7 +1719,7 @@ _is_block_dev()
> > >      if [ $# -ne 1 ]
> > >      then
> > >  	echo "Usage: _is_block_dev dev" 1>&2
> > > -	exit 1
> > > +	_exit 1
> > >      fi
> > >  
> > >      local dev=$1
> > > @@ -1739,7 +1738,7 @@ _is_char_dev()
> > >  {
> > >  	if [ $# -ne 1 ]; then
> > >  		echo "Usage: _is_char_dev dev" 1>&2
> > > -		exit 1
> > > +		_exit 1
> > >  	fi
> > >  
> > >  	local dev=$1
> > > @@ -1772,7 +1771,7 @@ _do()
> > >  	echo -n "$note... "
> > >      else
> > >  	echo "Usage: _do [note] cmd" 1>&2
> > > -	status=1; exit
> > > +	_exit 1
> > >      fi
> > >  
> > >      (eval "echo '---' \"$cmd\"") >>$seqres.full
> > > @@ -1793,7 +1792,7 @@ _do()
> > >      then
> > >  	[ $# -ne 2 ] && echo
> > >  	eval "echo \"$cmd\" failed \(returned $ret\): see $seqres.full"
> > > -	status=1; exit
> > > +	_exit 1
> > >      fi
> > >  
> > >      return $ret
> > > @@ -1809,8 +1808,7 @@ _notrun()
> > >      rm -f ${RESULT_DIR}/require_test*
> > >      rm -f ${RESULT_DIR}/require_scratch*
> > >  
> > > -    status=0
> > > -    exit
> > > +    _exit 0
> > >  }
> > >  
> > >  # just plain bail out
> > > @@ -1819,8 +1817,7 @@ _fail()
> > >  {
> > >      echo "$*" | tee -a $seqres.full
> > >      echo "(see $seqres.full for details)"
> > > -    status=1
> > > -    exit 1
> > > +    _exit 1
> > >  }
> > >  
> > >  #
> > > @@ -2049,14 +2046,14 @@ _require_scratch_nocheck()
> > >  
> > >      _check_mounted_on SCRATCH_DEV $SCRATCH_DEV SCRATCH_MNT $SCRATCH_MNT
> > >      local err=$?
> > > -    [ $err -le 1 ] || exit 1
> > > +    [ $err -le 1 ] || _exit 1
> > >      if [ $err -eq 0 ]
> > >      then
> > >          # if it's mounted, unmount it
> > >          if ! _scratch_unmount
> > >          then
> > >              echo "failed to unmount $SCRATCH_DEV"
> > > -            exit 1
> > > +            _exit 1
> > >          fi
> > >      fi
> > >      rm -f ${RESULT_DIR}/require_scratch "$RESULT_DIR/.skip_orebuild" "$RESULT_DIR/.skip_rebuild"
> > > @@ -2273,13 +2270,13 @@ _require_test()
> > >  
> > >      _check_mounted_on TEST_DEV $TEST_DEV TEST_DIR $TEST_DIR
> > >      local err=$?
> > > -    [ $err -le 1 ] || exit 1
> > > +    [ $err -le 1 ] || _exit 1
> > >      if [ $err -ne 0 ]
> > >      then
> > >  	if ! _test_mount
> > >  	then
> > >  		echo "!!! failed to mount $TEST_DEV on $TEST_DIR"
> > > -		exit 1
> > > +		_exit 1
> > >  	fi
> > >      fi
> > >      touch ${RESULT_DIR}/require_test
> > > @@ -2391,7 +2388,7 @@ _require_block_device()
> > >  {
> > >  	if [ -z "$1" ]; then
> > >  		echo "Usage: _require_block_device <dev>" 1>&2
> > > -		exit 1
> > > +		_exit 1
> > >  	fi
> > >  	if [ "`_is_block_dev "$1"`" == "" ]; then
> > >  		_notrun "require $1 to be valid block disk"
> > > @@ -2404,7 +2401,7 @@ _require_local_device()
> > >  {
> > >  	if [ -z "$1" ]; then
> > >  		echo "Usage: _require_local_device <dev>" 1>&2
> > > -		exit 1
> > > +		_exit 1
> > >  	fi
> > >  	if [ "`_is_block_dev "$1"`" != "" ]; then
> > >  		return 0
> > > @@ -2512,7 +2509,7 @@ _zone_type()
> > >  	local target=$1
> > >  	if [ -z $target ]; then
> > >  		echo "Usage: _zone_type <device>"
> > > -		exit 1
> > > +		_exit 1
> > >  	fi
> > >  	local sdev=`_short_dev $target`
> > >  
> > > @@ -2528,7 +2525,7 @@ _require_zoned_device()
> > >  	local target=$1
> > >  	if [ -z $target ]; then
> > >  		echo "Usage: _require_zoned_device <device>"
> > > -		exit 1
> > > +		_exit 1
> > >  	fi
> > >  
> > >  	local type=`_zone_type ${target}`
> > > @@ -2668,7 +2665,7 @@ _run_aiodio()
> > >      if [ -z "$1" ]
> > >      then
> > >          echo "usage: _run_aiodio command_name" 2>&1
> > > -        status=1; exit 1
> > > +        _exit 1
> > >      fi
> > >  
> > >      _require_aiodio $1
> > > @@ -2880,7 +2877,7 @@ _require_xfs_io_command()
> > >  	if [ -z "$1" ]
> > >  	then
> > >  		echo "Usage: _require_xfs_io_command command [switch]" 1>&2
> > > -		exit 1
> > > +		_exit 1
> > >  	fi
> > >  	local command=$1
> > >  	shift
> > > @@ -3364,7 +3361,7 @@ _is_dev_mounted()
> > >  
> > >  	if [ $# -lt 1 ]; then
> > >  		echo "Usage: _is_dev_mounted <device> [fstype]" 1>&2
> > > -		exit 1
> > > +		_exit 1
> > >  	fi
> > >  
> > >  	findmnt -rncv -S $dev -t $fstype -o TARGET | head -1
> > > @@ -3378,7 +3375,7 @@ _is_dir_mountpoint()
> > >  
> > >  	if [ $# -lt 1 ]; then
> > >  		echo "Uasge: _is_dir_mountpoint <dir> [fstype]" 1>&2
> > > -		exit 1
> > > +		_exit 1
> > >  	fi
> > >  
> > >  	findmnt -rncv -t $fstype -o TARGET $dir | head -1
> > > @@ -3391,7 +3388,7 @@ _remount()
> > >      if [ $# -ne 2 ]
> > >      then
> > >  	echo "Usage: _remount device ro/rw" 1>&2
> > > -	exit 1
> > > +	_exit 1
> > >      fi
> > >      local device=$1
> > >      local mode=$2
> > > @@ -3399,7 +3396,7 @@ _remount()
> > >      if ! mount -o remount,$mode $device
> > >      then
> > >          echo "_remount: failed to remount filesystem on $device as $mode"
> > > -        exit 1
> > > +        _exit 1
> > >      fi
> > >  }
> > >  
> > > @@ -3417,7 +3414,7 @@ _umount_or_remount_ro()
> > >      if [ $# -ne 1 ]
> > >      then
> > >  	echo "Usage: _umount_or_remount_ro <device>" 1>&2
> > > -	exit 1
> > > +	_exit 1
> > >      fi
> > >  
> > >      local device=$1
> > > @@ -3435,7 +3432,7 @@ _mount_or_remount_rw()
> > >  {
> > >  	if [ $# -ne 3 ]; then
> > >  		echo "Usage: _mount_or_remount_rw <opts> <dev> <mnt>" 1>&2
> > > -		exit 1
> > > +		_exit 1
> > >  	fi
> > >  	local mount_opts=$1
> > >  	local device=$2
> > > @@ -3516,7 +3513,7 @@ _check_generic_filesystem()
> > >      if [ $ok -eq 0 ]; then
> > >  	status=1
> > >  	if [ "$iam" != "check" ]; then
> > > -		exit 1
> > > +		_exit 1
> > >  	fi
> > >  	return 1
> > >      fi
> > > @@ -3582,7 +3579,7 @@ _check_udf_filesystem()
> > >      if [ $# -ne 1 -a $# -ne 2 ]
> > >      then
> > >  	echo "Usage: _check_udf_filesystem device [last_block]" 1>&2
> > > -	exit 1
> > > +	_exit 1
> > >      fi
> > >  
> > >      if [ ! -x $here/src/udf_test ]
> > > @@ -3776,7 +3773,7 @@ _get_os_name()
> > >  		echo 'linux'
> > >  	else
> > >  		echo Unknown operating system: `uname`
> > > -		exit
> > > +		_exit 1
> > >  	fi
> > >  }
> > >  
> > > @@ -3837,7 +3834,7 @@ _link_out_file()
> > >  _die()
> > >  {
> > >          echo $@
> > > -        exit 1
> > > +        _exit 1
> > >  }
> > >  
> > >  # convert urandom incompressible data to compressible text data
> > > @@ -3994,7 +3991,7 @@ _require_scratch_dev_pool()
> > >  		if _mount | grep -q $i; then
> > >  			if ! _unmount $i; then
> > >  		            echo "failed to unmount $i - aborting"
> > > -		            exit 1
> > > +		            _exit 1
> > >  		        fi
> > >  		fi
> > >  		# To help better debug when something fails, we remove
> > > @@ -4403,7 +4400,7 @@ _require_batched_discard()
> > >  {
> > >  	if [ $# -ne 1 ]; then
> > >  		echo "Usage: _require_batched_discard mnt_point" 1>&2
> > > -		exit 1
> > > +		_exit 1
> > >  	fi
> > >  	_require_fstrim
> > >  
> > > @@ -4630,7 +4627,7 @@ _require_chattr()
> > >  {
> > >  	if [ -z "$1" ]; then
> > >  		echo "Usage: _require_chattr <attr>"
> > > -		exit 1
> > > +		_exit 1
> > >  	fi
> > >  	local attribute=$1
> > >  
> > > @@ -4649,7 +4646,7 @@ _get_total_inode()
> > >  {
> > >  	if [ -z "$1" ]; then
> > >  		echo "Usage: _get_total_inode <mnt>"
> > > -		exit 1
> > > +		_exit 1
> > >  	fi
> > >  	local nr_inode;
> > >  	nr_inode=`$DF_PROG -i $1 | tail -1 | awk '{print $3}'`
> > > @@ -4660,7 +4657,7 @@ _get_used_inode()
> > >  {
> > >  	if [ -z "$1" ]; then
> > >  		echo "Usage: _get_used_inode <mnt>"
> > > -		exit 1
> > > +		_exit 1
> > >  	fi
> > >  	local nr_inode;
> > >  	nr_inode=`$DF_PROG -i $1 | tail -1 | awk '{print $4}'`
> > > @@ -4671,7 +4668,7 @@ _get_used_inode_percent()
> > >  {
> > >  	if [ -z "$1" ]; then
> > >  		echo "Usage: _get_used_inode_percent <mnt>"
> > > -		exit 1
> > > +		_exit 1
> > >  	fi
> > >  	local pct_inode;
> > >  	pct_inode=`$DF_PROG -i $1 | tail -1 | awk '{ print $6 }' | \
> > > @@ -4683,7 +4680,7 @@ _get_free_inode()
> > >  {
> > >  	if [ -z "$1" ]; then
> > >  		echo "Usage: _get_free_inode <mnt>"
> > > -		exit 1
> > > +		_exit 1
> > >  	fi
> > >  	local nr_inode;
> > >  	nr_inode=`$DF_PROG -i $1 | tail -1 | awk '{print $5}'`
> > > @@ -4696,7 +4693,7 @@ _get_available_space()
> > >  {
> > >  	if [ -z "$1" ]; then
> > >  		echo "Usage: _get_available_space <mnt>"
> > > -		exit 1
> > > +		_exit 1
> > >  	fi
> > >  	$DF_PROG -B 1 $1 | tail -n1 | awk '{ print $5 }'
> > >  }
> > > @@ -4707,7 +4704,7 @@ _get_total_space()
> > >  {
> > >  	if [ -z "$1" ]; then
> > >  		echo "Usage: _get_total_space <mnt>"
> > > -		exit 1
> > > +		_exit 1
> > >  	fi
> > >  	$DF_PROG -B 1 $1 | tail -n1 | awk '{ print $3 }'
> > >  }
> > > @@ -4952,7 +4949,7 @@ init_rc()
> > >  	if [ "$TEST_DEV" = ""  ]
> > >  	then
> > >  		echo "common/rc: Error: \$TEST_DEV is not set"
> > > -		exit 1
> > > +		_exit 1
> > >  	fi
> > >  
> > >  	# if $TEST_DEV is not mounted, mount it now as XFS
> > > @@ -4966,20 +4963,20 @@ init_rc()
> > >  			if ! _test_mount
> > >  			then
> > >  				echo "common/rc: could not mount $TEST_DEV on $TEST_DIR"
> > > -				exit 1
> > > +				_exit 1
> > >  			fi
> > >  		fi
> > >  	fi
> > >  
> > >  	# Sanity check that TEST partition is not mounted at another mount point
> > >  	# or as another fs type
> > > -	_check_mounted_on TEST_DEV $TEST_DEV TEST_DIR $TEST_DIR $FSTYP || exit 1
> > > +	_check_mounted_on TEST_DEV $TEST_DEV TEST_DIR $TEST_DIR $FSTYP || _exit 1
> > >  	if [ -n "$SCRATCH_DEV" ]; then
> > >  		# Sanity check that SCRATCH partition is not mounted at another
> > >  		# mount point, because it is about to be unmounted and formatted.
> > >  		# Another fs type for scratch is fine (bye bye old fs type).
> > >  		_check_mounted_on SCRATCH_DEV $SCRATCH_DEV SCRATCH_MNT $SCRATCH_MNT
> > > -		[ $? -le 1 ] || exit 1
> > > +		[ $? -le 1 ] || _exit 1
> > >  	fi
> > >  
> > >  	# Figure out if we need to add -F ("foreign", deprecated) option to xfs_io
> > > @@ -5029,7 +5026,7 @@ _get_file_block_size()
> > >  {
> > >  	if [ -z $1 ] || [ ! -d $1 ]; then
> > >  		echo "Missing mount point argument for _get_file_block_size"
> > > -		exit 1
> > > +		_exit 1
> > >  	fi
> > >  
> > >  	case "$FSTYP" in
> > > @@ -5076,7 +5073,7 @@ _get_block_size()
> > >  {
> > >  	if [ -z $1 ] || [ ! -d $1 ]; then
> > >  		echo "Missing mount point argument for _get_block_size"
> > > -		exit 1
> > > +		_exit 1
> > >  	fi
> > >  	stat -f -c %S $1
> > >  }
> > > @@ -5146,14 +5143,14 @@ _run_hugepage_fsx() {
> > >  	fi
> > >  	cat $tmp.hugepage_fsx
> > >  	rm -f $tmp.hugepage_fsx
> > > -	test $res -ne 0 && exit 1
> > > +	test $res -ne 0 && _exit 1
> > >  	return 0
> > >  }
> > >  
> > >  # run fsx or exit the test
> > >  run_fsx()
> > >  {
> > > -	_run_fsx "$@" || exit 1
> > > +	_run_fsx "$@" || _exit 1
> > >  }
> > >  
> > >  _require_statx()
> > > @@ -5318,7 +5315,7 @@ _get_max_file_size()
> > >  {
> > >  	if [ -z $1 ] || [ ! -d $1 ]; then
> > >  		echo "Missing mount point argument for _get_max_file_size"
> > > -		exit 1
> > > +		_exit 1
> > >  	fi
> > >  
> > >  	local mnt=$1
> > > diff --git a/common/repair b/common/repair
> > > index a79f9b2b..fd206f8e 100644
> > > --- a/common/repair
> > > +++ b/common/repair
> > > @@ -16,7 +16,7 @@ _zero_position()
> > >  		}'`
> > >  	if [ -z "$offset" -o -z "$length" ]; then
> > >  		echo "cannot calculate offset ($offset) or length ($length)"
> > > -		exit
> > > +		_exit 1
> > >  	fi
> > >  	length=`expr $length / 512`
> > >  	$here/src/devzero -v $value -b 1 -n $length -o $offset $SCRATCH_DEV \
> > > @@ -113,7 +113,7 @@ _filter_dd()
> > >  }
> > >  
> > >  # do some controlled corrupting & ensure repair recovers us
> > > -# 
> > > +#
> > >  _check_repair()
> > >  {
> > >  	value=$1
> > > diff --git a/common/xfs b/common/xfs
> > > index 81d568d3..96c15f3c 100644
> > > --- a/common/xfs
> > > +++ b/common/xfs
> > > @@ -553,7 +553,7 @@ _require_xfs_db_command()
> > >  {
> > >  	if [ $# -ne 1 ]; then
> > >  		echo "Usage: _require_xfs_db_command command" 1>&2
> > > -		exit 1
> > > +		_exit 1
> > >  	fi
> > >  	command=$1
> > >  
> > > @@ -789,7 +789,7 @@ _check_xfs_filesystem()
> > >  
> > >  	if [ $# -ne 3 ]; then
> > >  		echo "Usage: _check_xfs_filesystem device <logdev>|none <rtdev>|none" 1>&2
> > > -		exit 1
> > > +		_exit 1
> > >  	fi
> > >  
> > >  	extra_mount_options=""
> > > @@ -1014,7 +1014,7 @@ _check_xfs_filesystem()
> > >  	if [ $ok -eq 0 ]; then
> > >  		status=1
> > >  		if [ "$iam" != "check" ]; then
> > > -			exit 1
> > > +			_exit 1
> > >  		fi
> > >  		return 1
> > >  	fi
> > > @@ -1379,7 +1379,7 @@ _require_xfs_spaceman_command()
> > >  {
> > >  	if [ -z "$1" ]; then
> > >  		echo "Usage: _require_xfs_spaceman_command command [switch]" 1>&2
> > > -		exit 1
> > > +		_exit 1
> > >  	fi
> > >  	local command=$1
> > >  	shift
> > > -- 
> > > 2.34.1
> > > 
> > > 
> > 
> 
> 

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2025-04-10 16:29 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-04-09  7:00 [PATCH v4 0/6] Minor cleanups in common/ Nirjhar Roy (IBM)
2025-04-09  7:00 ` [PATCH v4 1/6] generic/749: Remove redundant sourcing of common/rc Nirjhar Roy (IBM)
2025-04-09  7:00 ` [PATCH v4 2/6] generic/367: Remove redundant sourcing of common/config Nirjhar Roy (IBM)
2025-04-09 15:21   ` Darrick J. Wong
2025-04-09  7:00 ` [PATCH v4 3/6] check: Remove redundant _test_mount in check Nirjhar Roy (IBM)
2025-04-09 15:21   ` Darrick J. Wong
2025-04-09  7:00 ` [PATCH v4 4/6] check,common{rc,preamble}: Decouple init_rc() call from sourcing common/rc Nirjhar Roy (IBM)
2025-04-09 15:22   ` Darrick J. Wong
2025-04-09  7:00 ` [PATCH v4 5/6] common/config: Introduce _exit wrapper around exit command Nirjhar Roy (IBM)
2025-04-09 15:23   ` Darrick J. Wong
2025-04-09  7:00 ` [PATCH v4 6/6] common: exit --> _exit Nirjhar Roy (IBM)
2025-04-09 15:24   ` Darrick J. Wong
2025-04-09 16:29     ` Nirjhar Roy (IBM)
2025-04-10 14:31     ` Zorro Lang
2025-04-10 16:29       ` Darrick J. Wong

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox