[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20250407161914.mfnqef2vqghgy3c2@dell-per750-06-vm-08.rhts.eng.pek2.redhat.com>
Date: Tue, 8 Apr 2025 00:19:14 +0800
From: Zorro Lang <zlang@...hat.com>
To: Ritesh Harjani <ritesh.list@...il.com>
Cc: "Nirjhar Roy (IBM)" <nirjhar.roy.lists@...il.com>,
fstests@...r.kernel.org, linux-ext4@...r.kernel.org,
linux-xfs@...r.kernel.org, ojaswin@...ux.ibm.com, djwong@...nel.org,
zlang@...nel.org, david@...morbit.com
Subject: Re: [PATCH v2 5/5] common: exit --> _exit
On Fri, Apr 04, 2025 at 10:34:47AM +0530, Ritesh Harjani wrote:
> "Nirjhar Roy (IBM)" <nirjhar.roy.lists@...il.com> writes:
>
> > Replace exit <return-val> with _exit <return-val> which
> > is introduced in the previous patch.
> >
> > Signed-off-by: Nirjhar Roy (IBM) <nirjhar.roy.lists@...il.com>
> > ---
> > common/btrfs | 6 +--
> > common/ceph | 2 +-
> > common/config | 7 ++--
> > common/ext4 | 2 +-
> > common/populate | 2 +-
> > common/preamble | 2 +-
> > common/punch | 12 +++---
> > common/rc | 103 +++++++++++++++++++++++-------------------------
> > common/xfs | 8 ++--
> > 9 files changed, 70 insertions(+), 74 deletions(-)
> >
> > diff --git a/common/btrfs b/common/btrfs
> > index a3b9c12f..3725632c 100644
> > --- a/common/btrfs
> > +++ b/common/btrfs
> > @@ -80,7 +80,7 @@ _require_btrfs_mkfs_feature()
> > {
> > if [ -z $1 ]; then
> > echo "Missing feature name argument for _require_btrfs_mkfs_feature"
> > - exit 1
> > + _exit 1
> > fi
> > feat=$1
> > $MKFS_BTRFS_PROG -O list-all 2>&1 | \
> > @@ -104,7 +104,7 @@ _require_btrfs_fs_feature()
> > {
> > if [ -z $1 ]; then
> > echo "Missing feature name argument for _require_btrfs_fs_feature"
> > - exit 1
> > + _exit 1
> > fi
> > feat=$1
> > modprobe btrfs > /dev/null 2>&1
> > @@ -214,7 +214,7 @@ _check_btrfs_filesystem()
> > if [ $ok -eq 0 ]; then
> > status=1
> > if [ "$iam" != "check" ]; then
> > - exit 1
> > + _exit 1
> > fi
> > return 1
> > fi
> > diff --git a/common/ceph b/common/ceph
> > index d6f24df1..df7a6814 100644
> > --- a/common/ceph
> > +++ b/common/ceph
> > @@ -14,7 +14,7 @@ _ceph_create_file_layout()
> >
> > if [ -e $fname ]; then
> > echo "File $fname already exists."
> > - exit 1
> > + _exit 1
> > fi
> > touch $fname
> > $SETFATTR_PROG -n ceph.file.layout \
> > diff --git a/common/config b/common/config
> > index eb6af35a..4c5435b7 100644
> > --- a/common/config
> > +++ b/common/config
> > @@ -123,8 +123,7 @@ set_mkfs_prog_path_with_opts()
> > _fatal()
> > {
> > echo "$*"
> > - status=1
> > - exit 1
> > + _exit 1
> > }
> >
> > export MKFS_PROG="$(type -P mkfs)"
> > @@ -868,7 +867,7 @@ get_next_config() {
> > echo "Warning: need to define parameters for host $HOST"
> > echo " or set variables:"
> > echo " $MC"
> > - exit 1
> > + _exit 1
> > fi
> >
> > _check_device TEST_DEV required $TEST_DEV
> > @@ -879,7 +878,7 @@ get_next_config() {
> > if [ ! -z "$SCRATCH_DEV_POOL" ]; then
> > if [ ! -z "$SCRATCH_DEV" ]; then
> > echo "common/config: Error: \$SCRATCH_DEV ($SCRATCH_DEV) should be unset when \$SCRATCH_DEV_POOL ($SCRATCH_DEV_POOL) is set"
> > - exit 1
> > + _exit 1
> > fi
> > SCRATCH_DEV=`echo $SCRATCH_DEV_POOL | awk '{print $1}'`
> > export SCRATCH_DEV
> > diff --git a/common/ext4 b/common/ext4
> > index e1b336d3..f88fa532 100644
> > --- a/common/ext4
> > +++ b/common/ext4
> > @@ -182,7 +182,7 @@ _require_scratch_ext4_feature()
> > {
> > if [ -z "$1" ]; then
> > echo "Usage: _require_scratch_ext4_feature feature"
> > - exit 1
> > + _exit 1
> > fi
> > $MKFS_EXT4_PROG -F $MKFS_OPTIONS -O "$1" \
> > $SCRATCH_DEV 512m >/dev/null 2>&1 \
> > diff --git a/common/populate b/common/populate
> > index 7352f598..50dc75d3 100644
> > --- a/common/populate
> > +++ b/common/populate
> > @@ -1003,7 +1003,7 @@ _fill_fs()
> >
> > if [ $# -ne 4 ]; then
> > echo "Usage: _fill_fs filesize dir blocksize switch_user"
> > - exit 1
> > + _exit 1
> > fi
> >
> > if [ $switch_user -eq 0 ]; then
> > diff --git a/common/preamble b/common/preamble
> > index c92e55bb..ba029a34 100644
> > --- a/common/preamble
> > +++ b/common/preamble
> > @@ -35,7 +35,7 @@ _begin_fstest()
> > {
> > if [ -n "$seq" ]; then
> > echo "_begin_fstest can only be called once!"
> > - exit 1
> > + _exit 1
> > fi
> >
> > seq=`basename $0`
> > diff --git a/common/punch b/common/punch
> > index 43ccab69..6567b9d1 100644
> > --- a/common/punch
> > +++ b/common/punch
> > @@ -172,16 +172,16 @@ _filter_fiemap_flags()
> > $AWK_PROG -e "$awk_script" | _coalesce_extents
> > }
> >
> > -# Filters fiemap output to only print the
> > +# Filters fiemap output to only print the
> > # file offset column and whether or not
> > # it is an extent or a hole
> > _filter_hole_fiemap()
> > {
> > $AWK_PROG '
> > $3 ~ /hole/ {
> > - print $1, $2, $3;
> > + print $1, $2, $3;
> > next;
> > - }
> > + }
> > $5 ~ /0x[[:xdigit:]]+/ {
> > print $1, $2, "extent";
> > }' |
> > @@ -225,7 +225,7 @@ _filter_bmap()
> > die_now()
> > {
> > status=1
> > - exit
> > + _exit
>
> Why not remove status=1 too and just do _exit 1 here too?
> Like how we have done at other places?
Yeah, nice catch! As the defination of _exit:
_exit()
{
status="$1"
exit "$status"
}
The
"
status=1
exit
"
should be equal to:
"
_exit 1
"
And "_exit" looks not make sense, due to it gives null to status.
Same problem likes below:
@@ -3776,7 +3773,7 @@ _get_os_name()
echo 'linux'
else
echo Unknown operating system: `uname`
- exit
+ _exit
The "_exit" without argument looks not make sense.
Thanks,
Zorro
>
> Rest looks good to me.
>
> -ritesh
>
Powered by blists - more mailing lists