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

From: Paul E. McKenney
Date: Thu Sep 27 2018 - 00:29:05 EST


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