* [PATCH v1 0/2] common: Move exit related functions to common/exit @ 2025-04-23 6:41 Nirjhar Roy (IBM) 2025-04-23 6:41 ` [PATCH v1 1/2] common: Move exit related functions to a common/exit Nirjhar Roy (IBM) 2025-04-23 6:41 ` [PATCH v1 2/2] check: Replace exit with _exit in check Nirjhar Roy (IBM) 0 siblings, 2 replies; 9+ messages in thread From: Nirjhar Roy (IBM) @ 2025-04-23 6:41 UTC (permalink / raw) To: fstests Cc: linux-ext4, linux-xfs, ritesh.list, ojaswin, djwong, zlang, david, nirjhar.roy.lists 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/Z_UJ7XcpmtkPRhTr@dread.disaster.area/ [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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v1 1/2] common: Move exit related functions to a common/exit 2025-04-23 6:41 [PATCH v1 0/2] common: Move exit related functions to common/exit Nirjhar Roy (IBM) @ 2025-04-23 6:41 ` Nirjhar Roy (IBM) 2025-04-23 14:18 ` Zorro Lang 2025-04-23 6:41 ` [PATCH v1 2/2] check: Replace exit with _exit in check Nirjhar Roy (IBM) 1 sibling, 1 reply; 9+ messages in thread From: Nirjhar Roy (IBM) @ 2025-04-23 6:41 UTC (permalink / raw) To: fstests Cc: linux-ext4, linux-xfs, ritesh.list, ojaswin, djwong, zlang, david, nirjhar.roy.lists 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@dread.disaster.area/ Suggested-by: Dave Chinner <david@fromorbit.com> Signed-off-by: Nirjhar Roy (IBM) <nirjhar.roy.lists@gmail.com> --- 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 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 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH v1 1/2] common: Move exit related functions to a common/exit 2025-04-23 6:41 ` [PATCH v1 1/2] common: Move exit related functions to a common/exit Nirjhar Roy (IBM) @ 2025-04-23 14:18 ` Zorro Lang 2025-04-24 9:09 ` Nirjhar Roy (IBM) 0 siblings, 1 reply; 9+ messages in thread From: Zorro Lang @ 2025-04-23 14:18 UTC (permalink / raw) To: Nirjhar Roy (IBM) Cc: fstests, linux-ext4, linux-xfs, ritesh.list, ojaswin, djwong, zlang, david 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@dread.disaster.area/ > Suggested-by: Dave Chinner <david@fromorbit.com> > Signed-off-by: Nirjhar Roy (IBM) <nirjhar.roy.lists@gmail.com> > --- > 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 > 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 > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v1 1/2] common: Move exit related functions to a common/exit 2025-04-23 14:18 ` Zorro Lang @ 2025-04-24 9:09 ` Nirjhar Roy (IBM) 2025-04-25 11:27 ` Zorro Lang 0 siblings, 1 reply; 9+ messages in thread From: Nirjhar Roy (IBM) @ 2025-04-24 9:09 UTC (permalink / raw) To: Zorro Lang Cc: fstests, linux-ext4, linux-xfs, ritesh.list, ojaswin, djwong, zlang, david 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@dread.disaster.area/ >> Suggested-by: Dave Chinner <david@fromorbit.com> >> Signed-off-by: Nirjhar Roy (IBM) <nirjhar.roy.lists@gmail.com> >> --- >> 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? --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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v1 1/2] common: Move exit related functions to a common/exit 2025-04-24 9:09 ` Nirjhar Roy (IBM) @ 2025-04-25 11:27 ` Zorro Lang 2025-04-25 12:03 ` Nirjhar Roy (IBM) 0 siblings, 1 reply; 9+ messages in thread From: Zorro Lang @ 2025-04-25 11:27 UTC (permalink / raw) To: Nirjhar Roy (IBM) Cc: fstests, linux-ext4, linux-xfs, ritesh.list, ojaswin, djwong, zlang, david 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@dread.disaster.area/ > > > Suggested-by: Dave Chinner <david@fromorbit.com> > > > Signed-off-by: Nirjhar Roy (IBM) <nirjhar.roy.lists@gmail.com> > > > --- > > > 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). 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 > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v1 1/2] common: Move exit related functions to a common/exit 2025-04-25 11:27 ` Zorro Lang @ 2025-04-25 12:03 ` Nirjhar Roy (IBM) 2025-04-25 13:36 ` Zorro Lang 0 siblings, 1 reply; 9+ messages in thread From: Nirjhar Roy (IBM) @ 2025-04-25 12:03 UTC (permalink / raw) To: Zorro Lang Cc: fstests, linux-ext4, linux-xfs, ritesh.list, ojaswin, djwong, zlang, david 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@dread.disaster.area/ >>>> Suggested-by: Dave Chinner <david@fromorbit.com> >>>> Signed-off-by: Nirjhar Roy (IBM) <nirjhar.roy.lists@gmail.com> >>>> --- >>>> 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@gmail.com/ --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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v1 1/2] common: Move exit related functions to a common/exit 2025-04-25 12:03 ` Nirjhar Roy (IBM) @ 2025-04-25 13:36 ` Zorro Lang 2025-04-28 8:39 ` Nirjhar Roy (IBM) 0 siblings, 1 reply; 9+ messages in thread From: Zorro Lang @ 2025-04-25 13:36 UTC (permalink / raw) To: Nirjhar Roy (IBM) Cc: fstests, linux-ext4, linux-xfs, ritesh.list, ojaswin, djwong, zlang, david On Fri, Apr 25, 2025 at 05:33:24PM +0530, Nirjhar Roy (IBM) wrote: > > 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@dread.disaster.area/ > > > > > Suggested-by: Dave Chinner <david@fromorbit.com> > > > > > Signed-off-by: Nirjhar Roy (IBM) <nirjhar.roy.lists@gmail.com> > > > > > --- > > > > > 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@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 :) > > --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 > ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH v1 1/2] common: Move exit related functions to a common/exit 2025-04-25 13:36 ` Zorro Lang @ 2025-04-28 8:39 ` Nirjhar Roy (IBM) 0 siblings, 0 replies; 9+ messages in thread From: Nirjhar Roy (IBM) @ 2025-04-28 8:39 UTC (permalink / raw) To: Zorro Lang Cc: fstests, linux-ext4, linux-xfs, ritesh.list, ojaswin, djwong, zlang, david On 4/25/25 19:06, Zorro Lang wrote: > On Fri, Apr 25, 2025 at 05:33:24PM +0530, Nirjhar Roy (IBM) wrote: >> 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@dread.disaster.area/ >>>>>> Suggested-by: Dave Chinner <david@fromorbit.com> >>>>>> Signed-off-by: Nirjhar Roy (IBM) <nirjhar.roy.lists@gmail.com> >>>>>> --- >>>>>> 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@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 :) 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 > >> --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 >> -- Nirjhar Roy Linux Kernel Developer IBM, Bangalore ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH v1 2/2] check: Replace exit with _exit in check 2025-04-23 6:41 [PATCH v1 0/2] common: Move exit related functions to common/exit Nirjhar Roy (IBM) 2025-04-23 6:41 ` [PATCH v1 1/2] common: Move exit related functions to a common/exit Nirjhar Roy (IBM) @ 2025-04-23 6:41 ` Nirjhar Roy (IBM) 1 sibling, 0 replies; 9+ messages in thread From: Nirjhar Roy (IBM) @ 2025-04-23 6:41 UTC (permalink / raw) To: fstests Cc: linux-ext4, linux-xfs, ritesh.list, ojaswin, djwong, zlang, david, nirjhar.roy.lists Some of the "status=<val>;exit" and "exit <val>" were not replace with _exit <val>. Doing it now. Signed-off-by: Nirjhar Roy (IBM) <nirjhar.roy.lists@gmail.com> --- check | 39 +++++++++++++++------------------------ 1 file changed, 15 insertions(+), 24 deletions(-) diff --git a/check b/check index 67355c52..30d44c0e 100755 --- a/check +++ b/check @@ -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` -- 2.34.1 ^ permalink raw reply related [flat|nested] 9+ messages in thread
end of thread, other threads:[~2025-04-28 8:39 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-04-23 6:41 [PATCH v1 0/2] common: Move exit related functions to common/exit Nirjhar Roy (IBM) 2025-04-23 6:41 ` [PATCH v1 1/2] common: Move exit related functions to a common/exit Nirjhar Roy (IBM) 2025-04-23 14:18 ` Zorro Lang 2025-04-24 9:09 ` Nirjhar Roy (IBM) 2025-04-25 11:27 ` Zorro Lang 2025-04-25 12:03 ` Nirjhar Roy (IBM) 2025-04-25 13:36 ` Zorro Lang 2025-04-28 8:39 ` Nirjhar Roy (IBM) 2025-04-23 6:41 ` [PATCH v1 2/2] check: Replace exit with _exit in check Nirjhar Roy (IBM)
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox