linux-ext4 - Re: [PATCH v3 5/6] common/config: Introduce

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Z_XC0sMObxxHOWM5@dread.disaster.area>
Date: Wed, 9 Apr 2025 10:44:02 +1000
From: Dave Chinner <david@...morbit.com>
To: "Nirjhar Roy (IBM)" <nirjhar.roy.lists@...il.com>
Cc: fstests@...r.kernel.org, linux-ext4@...r.kernel.org,
	linux-xfs@...r.kernel.org, ritesh.list@...il.com,
	ojaswin@...ux.ibm.com, djwong@...nel.org, zlang@...nel.org
Subject: Re: [PATCH v3 5/6] common/config: Introduce _exit wrapper around
 exit command

On Tue, Apr 08, 2025 at 10:13:54PM +0530, Nirjhar Roy (IBM) wrote:
> On 4/8/25 17:05, Dave Chinner wrote:
> > On Tue, Apr 08, 2025 at 05:37:21AM +0000, Nirjhar Roy (IBM) wrote:
> > > diff --git a/common/config b/common/config
> > > index 79bec87f..eb6af35a 100644
> > > --- a/common/config
> > > +++ b/common/config
> > > @@ -96,6 +96,14 @@ 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.
> > > +_exit()
> > > +{
> > > +	status="$1"
> > > +	exit "$status"
> > > +}
> > The only issue with putting this helper in common/config is that
> > calling _exit() requires sourcing common/config from the shell
> > context that calls it.
> > 
> > This means every test must source common/config and re-execute the
> > environment setup, even though we already have all the environment
> > set up because it was exported from check when it sourced
> > common/config.
> > 
> > We have the same problem with _fatal() - it is defined in
> > common/config instead of an independent common file. If we put such
> > functions in their own common file, they can be sourced
> > without dependencies on any other common file being included first.
> > 
> > e.g. we create common/exit and place all the functions that
> > terminate tests in it - _fatal, _notrun, _exit, etc and source that
> > file once per shell context before we source common/config,
> > common/rc, etc. This means we can source and call those termination
> > functions from any context without having to worry about
> > dependencies...
> 
> Yes, I agree to the above. Do you want this refactoring to be done as a part
> of this patch series in the further revisions, or can this be sent as a
> separate series?

Seperate series is fine. You're not making the dependency mess any
worse than it already is with this change...

-Dave.
-- 
Dave Chinner
david@...morbit.com