* [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 an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.