[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ee2234d0-d27d-45ad-b19d-419ac9126dee@gmail.com>
Date: Tue, 8 Apr 2025 00:29:59 +0530
From: "Nirjhar Roy (IBM)" <nirjhar.roy.lists@...il.com>
To: Zorro Lang <zlang@...hat.com>, Ritesh Harjani <ritesh.list@...il.com>
Cc: 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 4/7/25 21:49, Zorro Lang wrote:
> 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.
Yes, thank you Ritesh and Zorro. Yes it should have been "_exit 1". I
missed it while making the changes. I will make the changes in v3 and
add RBs from Dave[1] and Ritesh.
[1] https://lore.kernel.org/all/Z-xcne3f5Klvuxcq@dread.disaster.area/
>
> Thanks,
> Zorro
>
>> Rest looks good to me.
>>
>> -ritesh
>>
--
Nirjhar Roy
Linux Kernel Developer
IBM, Bangalore
Powered by blists - more mailing lists