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

Message-ID: <20180925194706.GA27944@GANOO-Loonix.attlocal.net>
Date:   Tue, 25 Sep 2018 14:47:08 -0500
From:   Pierce Griffiths <pierceagriffiths@...il.com>
To:     "Paul E. McKenney" <paulmck@...ux.ibm.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

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.

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
> > 
>