Re: [PATCH] torture-test modules: Remove unnecessary "ret" variables
From: Pierce Griffiths
Date: Tue Sep 25 2018 - 15:47:27 EST
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@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
> >
>