Skip to contentSkip to conversation

common: Move exit related functions to common/exit

PATCH Superseded Nirjhar Roy (IBM) 2 participants 9 messages Apr 23, 2025 – Apr 28, 2025

v1 of 3 Superseded Latest is v3 →

lore
Share & tools
lore
#0 lore raw

cover common: Move exit related functions to common/exit

This patch series moves all the exit related functions to a separate file - common/exit. This will remove the dependency to source non-related files to use these exit related functions. Thanks to Dave for suggesting this[1]. The second patch replaces exit with _exit in check file - I missed replacing them in [2].

[1] https://lore.kernel.org/all/[email protected]/
[2] https://lore.kernel.org/all/48dacdf636be19ae8bff66cc3852d27e28030613.1744181682.git.nirjhar.roy.lists@gmail.com/

Nirjhar Roy (IBM) (2):
common: Move exit related functions to a common/exit
check: Replace exit with _exit in check

 check           | 40 ++++++++++++++++-----------------------
 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 +
 13 files changed, 78 insertions(+), 76 deletions(-)
 create mode 100644 common/exit

--
2.34.1

#1 lore raw

PATCH 1/2 common: Move exit related functions to a common/exit

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/[email protected]/

---
 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 +
 13 files changed, 63 insertions(+), 52 deletions(-)
 create mode 100644 common/exit
check +1 -0
index 9451c350..67355c52 100755
@@ -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()
 {
common/btrfs +1 -1
index 3725632c..9e91ee71 100644
@@ -1,7 +1,7 @@
 #
 # Common btrfs specific functions
 #
-
+. common/exit
 . common/module
 
 # The recommended way to execute simple "btrfs" command.
common/ceph +2 -0
index df7a6814..89e36403 100644
@@ -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.
common/config +1 -16
index eada3971..6a60d144 100644
@@ -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"
 
common/dump +1 -0
index 09859006..4701a956 100644
@@ -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
common/exit new file +50 -0

50 lines not on this page · raw message

common/ext4 +1 -1
index f88fa532..ab566c41 100644
@@ -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
common/populate +1 -1
index 50dc75d3..a17acc9e 100644
@@ -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() {
common/preamble +1 -0
index ba029a34..0f306412 100644
@@ -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()
common/punch +1 -5
index 64d665d8..637f463f 100644
@@ -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
common/rc +1 -28
index 9bed6dad..945f5134 100644
@@ -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()
 {
common/repair +1 -0
index fd206f8e..db6a1b5c 100644
@@ -3,6 +3,7 @@
 # Copyright (c) 2000-2002 Silicon Graphics, Inc.  All Rights Reserved.
 #
 # Functions useful for xfs_repair tests
+. common/exit
 
 _zero_position()
 {
common/xfs +1 -0
index 96c15f3c..c236146c 100644
@@ -1,6 +1,7 @@
 #
 # XFS specific common functions.
 #
+. common/exit
 
 __generate_xfs_report_vars() {
 	__generate_blockdev_report_vars TEST_RTDEV
Signature · 1 line
2.34.1
#2 lore raw

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

Quoted 8 lines · Nirjhar Roy (IBM)

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/[email protected]/

Quoted 14 lines
---
 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?

Thanks,
Zorro

Quoted 294 lines
 13 files changed, 63 insertions(+), 52 deletions(-)
 create mode 100644 common/exit
check +1 -0
index 9451c350..67355c52 100755
@@ -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()
 {
common/btrfs +1 -1
index 3725632c..9e91ee71 100644
@@ -1,7 +1,7 @@
 #
 # Common btrfs specific functions
 #
-
+. common/exit
 . common/module
 
 # The recommended way to execute simple "btrfs" command.
common/ceph +2 -0
index df7a6814..89e36403 100644
@@ -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.
common/config +1 -16
index eada3971..6a60d144 100644
@@ -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"
 
common/dump +1 -0
index 09859006..4701a956 100644
@@ -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
common/exit new file +50 -0
new file mode 100644
index 00000000..ad7e7498
@@ -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
+}
+
common/ext4 +1 -1
index f88fa532..ab566c41 100644
@@ -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
common/populate +1 -1
index 50dc75d3..a17acc9e 100644
@@ -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() {
common/preamble +1 -0
index ba029a34..0f306412 100644
@@ -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()
common/punch +1 -5
index 64d665d8..637f463f 100644
@@ -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
common/rc +1 -28
index 9bed6dad..945f5134 100644
@@ -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()
 {
common/repair +1 -0
index fd206f8e..db6a1b5c 100644
@@ -3,6 +3,7 @@
 # Copyright (c) 2000-2002 Silicon Graphics, Inc.  All Rights Reserved.
 #
 # Functions useful for xfs_repair tests
+. common/exit
 
 _zero_position()
 {
common/xfs +1 -0
index 96c15f3c..c236146c 100644
@@ -1,6 +1,7 @@
 #
 # XFS specific common functions.
 #
+. common/exit
 
 __generate_xfs_report_vars() {
 	__generate_blockdev_report_vars TEST_RTDEV
Signature · 1 line
2.34.1
#3 lore raw

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

Quoted 23 lines · Zorro Lang
Quoted 8 lines · Nirjhar Roy (IBM)

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/[email protected]/

Quoted 14 lines
---
  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 +
Quoted 5 lines

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?

--NR

Quoted 298 lines

Thanks,
Zorro

Quoted 294 lines
  13 files changed, 63 insertions(+), 52 deletions(-)

create mode 100644 common/exit

check +1 -0
index 9451c350..67355c52 100755
@@ -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()
  {
common/btrfs +1 -1
index 3725632c..9e91ee71 100644
@@ -1,7 +1,7 @@
  #
  # Common btrfs specific functions
  #
-
+. common/exit
  . common/module
 
  # The recommended way to execute simple "btrfs" command.
common/ceph +2 -0
index df7a6814..89e36403 100644
@@ -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.
common/config +1 -16
index eada3971..6a60d144 100644
@@ -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"
 
common/dump +1 -0
index 09859006..4701a956 100644
@@ -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
common/exit new file +50 -0
new file mode 100644
index 00000000..ad7e7498
@@ -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
+}
+
common/ext4 +1 -1
index f88fa532..ab566c41 100644
@@ -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
common/populate +1 -1
index 50dc75d3..a17acc9e 100644
@@ -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() {
common/preamble +1 -0
index ba029a34..0f306412 100644
@@ -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()
common/punch +1 -5
index 64d665d8..637f463f 100644
@@ -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
common/rc +1 -28
index 9bed6dad..945f5134 100644
@@ -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()
  {
common/repair +1 -0
index fd206f8e..db6a1b5c 100644
@@ -3,6 +3,7 @@
  # Copyright (c) 2000-2002 Silicon Graphics, Inc.  All Rights Reserved.
  #
  # Functions useful for xfs_repair tests
+. common/exit
 
  _zero_position()
  {
common/xfs +1 -0
index 96c15f3c..c236146c 100644
@@ -1,6 +1,7 @@
  #
  # XFS specific common functions.
  #
+. common/exit
 
  __generate_xfs_report_vars() {
  	__generate_blockdev_report_vars TEST_RTDEV
Signature · 1 line
2.34.1
Signature · 3 lines
Nirjhar Roy
Linux Kernel Developer
IBM, Bangalore
#4 lore raw

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

Quoted 31 lines · Nirjhar Roy (IBM)
Quoted 23 lines · Zorro Lang
Quoted 8 lines · Nirjhar Roy (IBM)

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/[email protected]/

Quoted 14 lines
---
  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 +
Quoted 5 lines

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?

Quoted 5 lines

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).

Thanks,
Zorro

Quoted 285 lines

--NR

Quoted 277 lines

Thanks,
Zorro

Quoted 273 lines
  13 files changed, 63 insertions(+), 52 deletions(-)

create mode 100644 common/exit

check +1 -0
index 9451c350..67355c52 100755
@@ -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()
  {
common/btrfs +1 -1
index 3725632c..9e91ee71 100644
@@ -1,7 +1,7 @@
  #
  # Common btrfs specific functions
  #
-
+. common/exit
  . common/module
  # The recommended way to execute simple "btrfs" command.
common/ceph +2 -0
index df7a6814..89e36403 100644
@@ -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.
common/config +1 -16
index eada3971..6a60d144 100644
@@ -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"
common/dump +1 -0
index 09859006..4701a956 100644
@@ -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
common/exit new file +50 -0
new file mode 100644
index 00000000..ad7e7498
@@ -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
+}
+
common/ext4 +1 -1
index f88fa532..ab566c41 100644
@@ -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
common/populate +1 -1
index 50dc75d3..a17acc9e 100644
@@ -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() {
common/preamble +1 -0
index ba029a34..0f306412 100644
@@ -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()
common/punch +1 -5
index 64d665d8..637f463f 100644
@@ -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
common/rc +1 -28
index 9bed6dad..945f5134 100644
@@ -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()
  {
common/repair +1 -0
index fd206f8e..db6a1b5c 100644
@@ -3,6 +3,7 @@
  # Copyright (c) 2000-2002 Silicon Graphics, Inc.  All Rights Reserved.
  #
  # Functions useful for xfs_repair tests
+. common/exit
  _zero_position()
  {
common/xfs +1 -0
index 96c15f3c..c236146c 100644
@@ -1,6 +1,7 @@
  #
  # XFS specific common functions.
  #
+. common/exit
  __generate_xfs_report_vars() {
  	__generate_blockdev_report_vars TEST_RTDEV
Signature · 1 line
2.34.1
Signature · 3 lines
Nirjhar Roy
Linux Kernel Developer
IBM, Bangalore
#5 lore raw

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

Quoted 35 lines · Zorro Lang
Quoted 29 lines · Nirjhar Roy (IBM)
Quoted 23 lines · Zorro Lang
Quoted 8 lines · Nirjhar Roy (IBM)

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/[email protected]/

Quoted 14 lines
---
   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 +
Quoted 5 lines

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?

Quoted 5 lines

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?

Quoted 4 lines

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@gmail.com/

--NR

Quoted 287 lines

Thanks,
Zorro

Quoted 283 lines

--NR

Quoted 276 lines

Thanks,
Zorro

Quoted 273 lines
   13 files changed, 63 insertions(+), 52 deletions(-)

create mode 100644 common/exit

check +1 -0
index 9451c350..67355c52 100755
@@ -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()
   {
common/btrfs +1 -1
index 3725632c..9e91ee71 100644
@@ -1,7 +1,7 @@
   #
   # Common btrfs specific functions
   #
-
+. common/exit
   . common/module
   # The recommended way to execute simple "btrfs" command.
common/ceph +2 -0
index df7a6814..89e36403 100644
@@ -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.
common/config +1 -16
index eada3971..6a60d144 100644
@@ -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"
common/dump +1 -0
index 09859006..4701a956 100644
@@ -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
common/exit new file +50 -0
new file mode 100644
index 00000000..ad7e7498
@@ -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
+}
+
common/ext4 +1 -1
index f88fa532..ab566c41 100644
@@ -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
common/populate +1 -1
index 50dc75d3..a17acc9e 100644
@@ -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() {
common/preamble +1 -0
index ba029a34..0f306412 100644
@@ -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()
common/punch +1 -5
index 64d665d8..637f463f 100644
@@ -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
common/rc +1 -28
index 9bed6dad..945f5134 100644
@@ -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()
   {
common/repair +1 -0
index fd206f8e..db6a1b5c 100644
@@ -3,6 +3,7 @@
   # Copyright (c) 2000-2002 Silicon Graphics, Inc.  All Rights Reserved.
   #
   # Functions useful for xfs_repair tests
+. common/exit
   _zero_position()
   {
common/xfs +1 -0
index 96c15f3c..c236146c 100644
@@ -1,6 +1,7 @@
   #
   # XFS specific common functions.
   #
+. common/exit
   __generate_xfs_report_vars() {
   	__generate_blockdev_report_vars TEST_RTDEV
Signature · 1 line
2.34.1
Signature · 3 lines
Nirjhar Roy
Linux Kernel Developer
IBM, Bangalore
Signature · 3 lines
Nirjhar Roy
Linux Kernel Developer
IBM, Bangalore
#6 lore raw

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

Quoted 42 lines · Nirjhar Roy (IBM)
Quoted 35 lines · Zorro Lang
Quoted 29 lines · Nirjhar Roy (IBM)
Quoted 23 lines · Zorro Lang
Quoted 8 lines · Nirjhar Roy (IBM)

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/[email protected]/

Quoted 14 lines
---
   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 +
Quoted 5 lines

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?

Quoted 5 lines

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?

Quoted 4 lines

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).

Quoted 6 lines

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@gmail.com/

Oh, as "check" has:

if $OPTIONS_HAVE_SECTIONS; then

          trap "_summary; exit \$status" 0 1 2 3 15
  else
          trap "_wrapup; exit \$status" 0 1 2 3 15

fi

So I think it makes sense to use _exit() to deal with status variable :)

Quoted 295 lines

--NR

Quoted 287 lines

Thanks,
Zorro

Quoted 283 lines

--NR

Quoted 276 lines

Thanks,
Zorro

Quoted 273 lines
   13 files changed, 63 insertions(+), 52 deletions(-)

create mode 100644 common/exit

check +1 -0
index 9451c350..67355c52 100755
@@ -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()
   {
common/btrfs +1 -1
index 3725632c..9e91ee71 100644
@@ -1,7 +1,7 @@
   #
   # Common btrfs specific functions
   #
-
+. common/exit
   . common/module
   # The recommended way to execute simple "btrfs" command.
common/ceph +2 -0
index df7a6814..89e36403 100644
@@ -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.
common/config +1 -16
index eada3971..6a60d144 100644
@@ -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"
common/dump +1 -0
index 09859006..4701a956 100644
@@ -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
common/exit new file +50 -0
new file mode 100644
index 00000000..ad7e7498
@@ -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
+}
+
common/ext4 +1 -1
index f88fa532..ab566c41 100644
@@ -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
common/populate +1 -1
index 50dc75d3..a17acc9e 100644
@@ -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() {
common/preamble +1 -0
index ba029a34..0f306412 100644
@@ -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()
common/punch +1 -5
index 64d665d8..637f463f 100644
@@ -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
common/rc +1 -28
index 9bed6dad..945f5134 100644
@@ -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()
   {
common/repair +1 -0
index fd206f8e..db6a1b5c 100644
@@ -3,6 +3,7 @@
   # Copyright (c) 2000-2002 Silicon Graphics, Inc.  All Rights Reserved.
   #
   # Functions useful for xfs_repair tests
+. common/exit
   _zero_position()
   {
common/xfs +1 -0
index 96c15f3c..c236146c 100644
@@ -1,6 +1,7 @@
   #
   # XFS specific common functions.
   #
+. common/exit
   __generate_xfs_report_vars() {
   	__generate_blockdev_report_vars TEST_RTDEV
Signature · 1 line
2.34.1
Signature · 3 lines
Nirjhar Roy
Linux Kernel Developer
IBM, Bangalore
Signature · 3 lines
Nirjhar Roy
Linux Kernel Developer
IBM, Bangalore
#7 lore raw

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

Quoted 49 lines · Zorro Lang
Quoted 40 lines · Nirjhar Roy (IBM)
Quoted 35 lines · Zorro Lang
Quoted 29 lines · Nirjhar Roy (IBM)
Quoted 23 lines · Zorro Lang
Quoted 8 lines · Nirjhar Roy (IBM)

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/[email protected]/

Quoted 14 lines
---
    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 +
Quoted 5 lines

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?

Quoted 5 lines

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?

Quoted 4 lines

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).

Quoted 6 lines

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@gmail.com/

Oh, as "check" has:

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

So I think it makes sense to use _exit() to deal with status variable :)

Oh, right. Yes, I can replace this "exit \$status" with "_exit". I will make the changes in v2. Any thoughts on the next patch[2]?

[2] https://lore.kernel.org/all/7d8587b8342ee2cbe226fb691b372ac7df5fdb71.1745390030.git.nirjhar.roy.lists@gmail.com/

--NR

Quoted 294 lines
Quoted 294 lines

--NR

Quoted 286 lines

Thanks,
Zorro

Quoted 283 lines

--NR

Quoted 276 lines

Thanks,
Zorro

Quoted 273 lines
    13 files changed, 63 insertions(+), 52 deletions(-)

create mode 100644 common/exit

check +1 -0
index 9451c350..67355c52 100755
@@ -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()
    {
common/btrfs +1 -1
index 3725632c..9e91ee71 100644
@@ -1,7 +1,7 @@
    #
    # Common btrfs specific functions
    #
-
+. common/exit
    . common/module
    # The recommended way to execute simple "btrfs" command.
common/ceph +2 -0
index df7a6814..89e36403 100644
@@ -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.
common/config +1 -16
index eada3971..6a60d144 100644
@@ -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"
common/dump +1 -0
index 09859006..4701a956 100644
@@ -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
common/exit new file +50 -0
new file mode 100644
index 00000000..ad7e7498
@@ -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
+}
+
common/ext4 +1 -1
index f88fa532..ab566c41 100644
@@ -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
common/populate +1 -1
index 50dc75d3..a17acc9e 100644
@@ -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() {
common/preamble +1 -0
index ba029a34..0f306412 100644
@@ -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()
common/punch +1 -5
index 64d665d8..637f463f 100644
@@ -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
common/rc +1 -28
index 9bed6dad..945f5134 100644
@@ -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()
    {
common/repair +1 -0
index fd206f8e..db6a1b5c 100644
@@ -3,6 +3,7 @@
    # Copyright (c) 2000-2002 Silicon Graphics, Inc.  All Rights Reserved.
    #
    # Functions useful for xfs_repair tests
+. common/exit
    _zero_position()
    {
common/xfs +1 -0
index 96c15f3c..c236146c 100644
@@ -1,6 +1,7 @@
    #
    # XFS specific common functions.
    #
+. common/exit
    __generate_xfs_report_vars() {
    	__generate_blockdev_report_vars TEST_RTDEV
Signature · 1 line
2.34.1
Signature · 3 lines
Nirjhar Roy
Linux Kernel Developer
IBM, Bangalore
Signature · 3 lines
Nirjhar Roy
Linux Kernel Developer
IBM, Bangalore
Signature · 3 lines
Nirjhar Roy
Linux Kernel Developer
IBM, Bangalore
#8 lore raw

PATCH 2/2 check: Replace exit with _exit in check

In reply to Nirjhar Roy (IBM)

Some of the "status=<val>;exit" and "exit <val>" were not replace with _exit <val>. Doing it now.

---
 check | 39 +++++++++++++++------------------------
 1 file changed, 15 insertions(+), 24 deletions(-)
check +15 -24
index 67355c52..30d44c0e 100755
@@ -122,7 +122,7 @@ examples:
  check -X .exclude -g auto
  check -E ~/.xfstests.exclude
 '
-	    exit 1
+	    _exit 1
 }
 
 get_sub_group_list()
@@ -232,7 +232,7 @@ _prepare_test_list()
 			list=$(get_group_list $group)
 			if [ -z "$list" ]; then
 				echo "Group \"$group\" is empty or not defined?"
-				exit 1
+				_exit 1
 			fi
 
 			for t in $list; do
@@ -317,14 +317,14 @@ while [ $# -gt 0 ]; do
 	-r)
 		if $exact_order; then
 			echo "Cannot specify -r and --exact-order."
-			exit 1
+			_exit 1
 		fi
 		randomize=true
 		;;
 	--exact-order)
 		if $randomize; then
 			echo "Cannnot specify --exact-order and -r."
-			exit 1
+			_exit 1
 		fi
 		exact_order=true
 		;;
@@ -362,7 +362,7 @@ done
 # after processing args, overlay needs FSTYP set before sourcing common/config
 if ! . ./common/rc; then
 	echo "check: failed to source common/rc"
-	exit 1
+	_exit 1
 fi
 
 init_rc
@@ -374,8 +374,7 @@ if [ -n "$SOAK_DURATION" ]; then
 		sed -e 's/^\([.0-9]*\)\([a-z]\)*/\1 \2/g' | \
 		$AWK_PROG -f $here/src/soak_duration.awk)"
 	if [ $? -ne 0 ]; then
-		status=1
-		exit 1
+		_exit 1
 	fi
 fi
 
@@ -386,8 +385,7 @@ if [ -n "$FUZZ_REWRITE_DURATION" ]; then
 		sed -e 's/^\([.0-9]*\)\([a-z]\)*/\1 \2/g' | \
 		$AWK_PROG -f $here/src/soak_duration.awk)"
 	if [ $? -ne 0 ]; then
-		status=1
-		exit 1
+		_exit 1
 	fi
 fi
 
@@ -405,8 +403,7 @@ if $have_test_arg; then
 	while [ $# -gt 0 ]; do
 		case "$1" in
 		-*)	echo "Arguments before tests, please!"
-			status=1
-			exit $status
+			_exit 1
 			;;
 		*)	# Expand test pattern (e.g. xfs/???, *fs/001)
 			list=$(cd $SRC_DIR; echo $1)
@@ -439,7 +436,7 @@ fi
 if [ `id -u` -ne 0 ]
 then
     echo "check: QA must be run as root"
-    exit 1
+    _exit 1
 fi
 
 _wipe_counters()
@@ -768,8 +765,7 @@ function run_section()
 	mkdir -p $RESULT_BASE
 	if [ ! -d $RESULT_BASE ]; then
 		echo "failed to create results directory $RESULT_BASE"
-		status=1
-		exit
+		_exit 1
 	fi
 
 	if $OPTIONS_HAVE_SECTIONS; then
@@ -785,8 +781,7 @@ function run_section()
 			echo "our local _test_mkfs routine ..."
 			cat $tmp.err
 			echo "check: failed to mkfs \$TEST_DEV using specified options"
-			status=1
-			exit
+			_exit 1
 		fi
 		# Previous FSTYP derived from TEST_DEV could be changed, source
 		# common/rc again with correct FSTYP to get FSTYP specific configs,
@@ -830,8 +825,7 @@ function run_section()
 	      echo "our local _scratch_mkfs routine ..."
 	      cat $tmp.err
 	      echo "check: failed to mkfs \$SCRATCH_DEV using specified options"
-	      status=1
-	      exit
+	      _exit 1
 	  fi
 
 	  # call the overridden mount - make sure the FS mounts with
@@ -841,8 +835,7 @@ function run_section()
 	      echo "our local mount routine ..."
 	      cat $tmp.err
 	      echo "check: failed to mount \$SCRATCH_DEV using specified options"
-	      status=1
-	      exit
+	      _exit 1
 	  else
 	      _scratch_unmount
 	  fi
@@ -1105,12 +1098,10 @@ for ((iters = 0; iters < $iterations; iters++)) do
 		run_section $section
 		if [ "$sum_bad" != 0 ] && [ "$istop" = true ]; then
 			interrupt=false
-			status=`expr $sum_bad != 0`
-			exit
+			_exit `expr $sum_bad != 0`
 		fi
 	done
 done
 
 interrupt=false
-status=`expr $sum_bad != 0`
-exit
+_exit `expr $sum_bad != 0`
Signature · 1 line
2.34.1