linux-kernel - Re: [PATCH] torture-test modules: Remove unnecessary "ret" variables

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] [day] [month] [year] [list]

Message-Id: <20180926220926.GM4222@linux.ibm.com>
Date:   Wed, 26 Sep 2018 15:09:26 -0700
From:   "Paul E. McKenney" <paulmck@...ux.ibm.com>
To:     Pierce Griffiths <pierceagriffiths@...il.com>
Cc:     Davidlohr Bueso <dave@...olabs.net>,
        Josh Triplett <josh@...htriplett.org>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] torture-test modules: Remove unnecessary "ret" variables

On Tue, Sep 25, 2018 at 02:47:08PM -0500, Pierce Griffiths wrote:
> Paul,
> The #ifdef cannot be replaced, as this would break the build for
> CONFIG_HOTPLUG_CPU=n builds. There are assignments and references to two
> global variables, a struct, and a function that are declared/defined in
> a large "#ifdef CONFIG_HOTPLUG_CPU" block which ends directly above
> torture_onoff_init(). IS_ENABLED() would cause these references to be
> present at compile time, but not their declarations or definitions,
> which would certainly cause a compilation error.

Good point, and thank you!  I have queued the original patch with a
minor edit to the Subject line.  It will make it to my -rcu tree once
I find a decent internet connection.

							Thanx, Paul

> On Sat, Sep 22, 2018 at 02:26:30PM -0700, Paul E. McKenney wrote:
> > On Fri, Sep 21, 2018 at 08:21:31PM -0500, Pierce Griffiths wrote:
> > > Remove return variables (declared as "ret") in cases where,
> > > depending on whether a condition evaluates as true, the result of a
> > > function call can be immediately returned instead of storing the result in
> > > the return variable. When the condition evaluates as false, the constant
> > > initially stored in the return variable at declaration is returned instead.
> > > 
> > > Signed-off-by: Pierce Griffiths <pierceagriffiths@...il.com>
> > 
> > Not bad, as it does safe a couple of lines.  Is it possible to save
> > a few more by using "if (!IS_ENABLED(CONFIG_HOTPLUG_CPU))" instead
> > of the #ifdef?  Or does that end up breaking the build for either
> > CONFIG_HOTPLUG_CPU=y or CONFIG_HOTPLUG_CPU=n builds?
> > 
> > 							Thanx, Paul
> > 
> > > ---
> > >  kernel/torture.c | 22 ++++++++--------------
> > >  1 file changed, 8 insertions(+), 14 deletions(-)
> > > 
> > > diff --git a/kernel/torture.c b/kernel/torture.c
> > > index 1ac24a826589..f4cec6db7f3c 100644
> > > --- a/kernel/torture.c
> > > +++ b/kernel/torture.c
> > > @@ -233,16 +233,15 @@ torture_onoff(void *arg)
> > >   */
> > >  int torture_onoff_init(long ooholdoff, long oointerval)
> > >  {
> > > -	int ret = 0;
> > > -
> > >  #ifdef CONFIG_HOTPLUG_CPU
> > >  	onoff_holdoff = ooholdoff;
> > >  	onoff_interval = oointerval;
> > >  	if (onoff_interval <= 0)
> > >  		return 0;
> > > -	ret = torture_create_kthread(torture_onoff, NULL, onoff_task);
> > > -#endif /* #ifdef CONFIG_HOTPLUG_CPU */
> > > -	return ret;
> > > +	return torture_create_kthread(torture_onoff, NULL, onoff_task);
> > > +#else /* #ifdef CONFIG_HOTPLUG_CPU */
> > > +	return 0;
> > > +#endif /* #else #ifdef CONFIG_HOTPLUG_CPU */
> > >  }
> > >  EXPORT_SYMBOL_GPL(torture_onoff_init);
> > > 
> > > @@ -513,15 +512,13 @@ static int torture_shutdown(void *arg)
> > >   */
> > >  int torture_shutdown_init(int ssecs, void (*cleanup)(void))
> > >  {
> > > -	int ret = 0;
> > > -
> > >  	torture_shutdown_hook = cleanup;
> > >  	if (ssecs > 0) {
> > >  		shutdown_time = ktime_add(ktime_get(), ktime_set(ssecs, 0));
> > > -		ret = torture_create_kthread(torture_shutdown, NULL,
> > > +		return torture_create_kthread(torture_shutdown, NULL,
> > >  					     shutdown_task);
> > >  	}
> > > -	return ret;
> > > +	return 0;
> > >  }
> > >  EXPORT_SYMBOL_GPL(torture_shutdown_init);
> > > 
> > > @@ -619,13 +616,10 @@ static int torture_stutter(void *arg)
> > >  /*
> > >   * Initialize and kick off the torture_stutter kthread.
> > >   */
> > > -int torture_stutter_init(int s)
> > > +int torture_stutter_init(const int s)
> > >  {
> > > -	int ret;
> > > -
> > >  	stutter = s;
> > > -	ret = torture_create_kthread(torture_stutter, NULL, stutter_task);
> > > -	return ret;
> > > +	return torture_create_kthread(torture_stutter, NULL, stutter_task);
> > >  }
> > >  EXPORT_SYMBOL_GPL(torture_stutter_init);
> > > 
> > > -- 
> > > 2.19.0
> > > 
> > 
>