Re: [PATCH v1 1/2] common: Move exit related functions to a common/exit

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On 4/25/25 16:57, Zorro Lang wrote:
On Thu, Apr 24, 2025 at 02:39:39PM +0530, Nirjhar Roy (IBM) wrote:
On 4/23/25 19:48, Zorro Lang wrote:
On Wed, Apr 23, 2025 at 06:41:34AM +0000, Nirjhar Roy (IBM) wrote:
Introduce a new file common/exit that will contain all the exit
related functions. This will remove the dependencies these functions
have on other non-related helper files and they can be indepedently
sourced. This was suggested by Dave Chinner[1].

[1] https://lore.kernel.org/linux-xfs/Z_UJ7XcpmtkPRhTr@xxxxxxxxxxxxxxxxxxx/
Suggested-by: Dave Chinner <david@xxxxxxxxxxxxx>
Signed-off-by: Nirjhar Roy (IBM) <nirjhar.roy.lists@xxxxxxxxx>
---
   check           |  1 +
   common/btrfs    |  2 +-
   common/ceph     |  2 ++
   common/config   | 17 +----------------
   common/dump     |  1 +
   common/exit     | 50 +++++++++++++++++++++++++++++++++++++++++++++++++
   common/ext4     |  2 +-
   common/populate |  2 +-
   common/preamble |  1 +
   common/punch    |  6 +-----
   common/rc       | 29 +---------------------------
   common/repair   |  1 +
   common/xfs      |  1 +
I think if you define exit helpers in common/exit, and import common/exit
in common/config, then you don't need to source it(common/exit) in other
common files (.e.g common/xfs, common/rc, etc). Due to when we call the
helpers in these common files, the process should already imported
common/rc -> common/config -> common/exit. right?
Oh, right. I can remove the redundant imports from
common/{btrfs,ceph,dump,ext4,populate,preamble,punch,rc,repair,xfs} in v2. I
will keep ". common/exit" only in common/config and check. The reason for me
to keep it in check is that before common/rc is sourced in check, we might
need _exit() (which is present is common/exit). Do you agree?
I thought "check" might not need that either. I didn't give it a test, but I found
before importing common/rc, there're only command arguments initialization, and
"check" calls "exit" directly if the initialization fails (except you want to call
_exit, but I didn't see you change that).

Yes, I have changed the exit() to _exit() in "check" in the next patch [1] of this series. Can you please take a look at that patch[1] and suggest whether I should have ". common/exit" in "check" or not?


[1] https://lore.kernel.org/all/7d8587b8342ee2cbe226fb691b372ac7df5fdb71.1745390030.git.nirjhar.roy.lists@xxxxxxxxx/

--NR


Thanks,
Zorro

--NR

Thanks,
Zorro

   13 files changed, 63 insertions(+), 52 deletions(-)
   create mode 100644 common/exit

diff --git a/check b/check
index 9451c350..67355c52 100755
--- a/check
+++ b/check
@@ -51,6 +51,7 @@ rm -f $tmp.list $tmp.tmp $tmp.grep $here/$iam.out $tmp.report.* $tmp.arglist
   SRC_GROUPS="generic"
   export SRC_DIR="tests"
+. common/exit
   usage()
   {
diff --git a/common/btrfs b/common/btrfs
index 3725632c..9e91ee71 100644
--- a/common/btrfs
+++ b/common/btrfs
@@ -1,7 +1,7 @@
   #
   # Common btrfs specific functions
   #
-
+. common/exit
   . common/module
   # The recommended way to execute simple "btrfs" command.
diff --git a/common/ceph b/common/ceph
index df7a6814..89e36403 100644
--- a/common/ceph
+++ b/common/ceph
@@ -2,6 +2,8 @@
   # CephFS specific common functions.
   #
+. common/exit
+
   # _ceph_create_file_layout <filename> <stripe unit> <stripe count> <object size>
   # This function creates a new empty file and sets the file layout according to
   # parameters.  It will exit if the file already exists.
diff --git a/common/config b/common/config
index eada3971..6a60d144 100644
--- a/common/config
+++ b/common/config
@@ -38,7 +38,7 @@
   # - this script shouldn't make any assertions about filesystem
   #   validity or mountedness.
   #
-
+. common/exit
   . common/test_names
   # all tests should use a common language setting to prevent golden
@@ -96,15 +96,6 @@ 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()
   {
@@ -121,12 +112,6 @@ set_mkfs_prog_path_with_opts()
   	fi
   }
-_fatal()
-{
-    echo "$*"
-    _exit 1
-}
-
   export MKFS_PROG="$(type -P mkfs)"
   [ "$MKFS_PROG" = "" ] && _fatal "mkfs not found"
diff --git a/common/dump b/common/dump
index 09859006..4701a956 100644
--- a/common/dump
+++ b/common/dump
@@ -3,6 +3,7 @@
   # Copyright (c) 2000-2002,2005 Silicon Graphics, Inc.  All Rights Reserved.
   #
   # Functions useful for xfsdump/xfsrestore tests
+. common/exit
   # --- initializations ---
   rm -f $seqres.full
diff --git a/common/exit b/common/exit
new file mode 100644
index 00000000..ad7e7498
--- /dev/null
+++ b/common/exit
@@ -0,0 +1,50 @@
+##/bin/bash
+
+# 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"
+}
+
+_fatal()
+{
+    echo "$*"
+    _exit 1
+}
+
+_die()
+{
+        echo $@
+        _exit 1
+}
+
+die_now()
+{
+	_exit 1
+}
+
+# just plain bail out
+#
+_fail()
+{
+    echo "$*" | tee -a $seqres.full
+    echo "(see $seqres.full for details)"
+    _exit 1
+}
+
+# bail out, setting up .notrun file. Need to kill the filesystem check files
+# here, otherwise they are set incorrectly for the next test.
+#
+_notrun()
+{
+    echo "$*" > $seqres.notrun
+    echo "$seq not run: $*"
+    rm -f ${RESULT_DIR}/require_test*
+    rm -f ${RESULT_DIR}/require_scratch*
+
+    _exit 0
+}
+
diff --git a/common/ext4 b/common/ext4
index f88fa532..ab566c41 100644
--- a/common/ext4
+++ b/common/ext4
@@ -1,7 +1,7 @@
   #
   # ext4 specific common functions
   #
-
+. common/exit
   __generate_ext4_report_vars() {
   	__generate_blockdev_report_vars TEST_LOGDEV
   	__generate_blockdev_report_vars SCRATCH_LOGDEV
diff --git a/common/populate b/common/populate
index 50dc75d3..a17acc9e 100644
--- a/common/populate
+++ b/common/populate
@@ -4,7 +4,7 @@
   #
   # Routines for populating a scratch fs, and helpers to exercise an FS
   # once it's been fuzzed.
-
+. common/exit
   . ./common/quota
   _require_populate_commands() {
diff --git a/common/preamble b/common/preamble
index ba029a34..0f306412 100644
--- a/common/preamble
+++ b/common/preamble
@@ -3,6 +3,7 @@
   # Copyright (c) 2021 Oracle.  All Rights Reserved.
   # Boilerplate fstests functionality
+. common/exit
   # Standard cleanup function.  Individual tests can override this.
   _cleanup()
diff --git a/common/punch b/common/punch
index 64d665d8..637f463f 100644
--- a/common/punch
+++ b/common/punch
@@ -3,6 +3,7 @@
   # Copyright (c) 2007 Silicon Graphics, Inc.  All Rights Reserved.
   #
   # common functions for excersizing hole punches with extent size hints etc.
+. common/exit
   _spawn_test_file() {
   	echo "# spawning test file with $*"
@@ -222,11 +223,6 @@ _filter_bmap()
   	_coalesce_extents
   }
-die_now()
-{
-	_exit 1
-}
-
   # test the different corner cases for zeroing a range:
   #
   #	1. into a hole
diff --git a/common/rc b/common/rc
index 9bed6dad..945f5134 100644
--- a/common/rc
+++ b/common/rc
@@ -2,6 +2,7 @@
   # SPDX-License-Identifier: GPL-2.0+
   # Copyright (c) 2000-2006 Silicon Graphics, Inc.  All Rights Reserved.
+. common/exit
   . common/config
   BC="$(type -P bc)" || BC=
@@ -1798,28 +1799,6 @@ _do()
       return $ret
   }
-# bail out, setting up .notrun file. Need to kill the filesystem check files
-# here, otherwise they are set incorrectly for the next test.
-#
-_notrun()
-{
-    echo "$*" > $seqres.notrun
-    echo "$seq not run: $*"
-    rm -f ${RESULT_DIR}/require_test*
-    rm -f ${RESULT_DIR}/require_scratch*
-
-    _exit 0
-}
-
-# just plain bail out
-#
-_fail()
-{
-    echo "$*" | tee -a $seqres.full
-    echo "(see $seqres.full for details)"
-    _exit 1
-}
-
   #
   # Tests whether $FSTYP should be exclude from this test.
   #
@@ -3835,12 +3814,6 @@ _link_out_file()
   	_link_out_file_named $seqfull.out "$features"
   }
-_die()
-{
-        echo $@
-        _exit 1
-}
-
   # convert urandom incompressible data to compressible text data
   _ddt()
   {
diff --git a/common/repair b/common/repair
index fd206f8e..db6a1b5c 100644
--- a/common/repair
+++ b/common/repair
@@ -3,6 +3,7 @@
   # Copyright (c) 2000-2002 Silicon Graphics, Inc.  All Rights Reserved.
   #
   # Functions useful for xfs_repair tests
+. common/exit
   _zero_position()
   {
diff --git a/common/xfs b/common/xfs
index 96c15f3c..c236146c 100644
--- a/common/xfs
+++ b/common/xfs
@@ -1,6 +1,7 @@
   #
   # XFS specific common functions.
   #
+. common/exit
   __generate_xfs_report_vars() {
   	__generate_blockdev_report_vars TEST_RTDEV
--
2.34.1

--
Nirjhar Roy
Linux Kernel Developer
IBM, Bangalore

--
Nirjhar Roy
Linux Kernel Developer
IBM, Bangalore





[Index of Archives]     [Reiser Filesystem Development]     [Ceph FS]     [Kernel Newbies]     [Security]     [Netfilter]     [Bugtraq]     [Linux FS]     [Yosemite National Park]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Device Mapper]     [Linux Media]

  Powered by Linux