[PATCH 2/2] driver core: fix statics initilisation

[PATCH 2/2] driver core: fix statics initilisation

[Willy Wolff]

perl scripts/checkpatch.pl -f drivers/base/dd.c got red here:

ERROR: do not initialise statics to false
+static bool driver_deferred_probe_enable = false;

Signed-off-by: Willy Wolff <willy.mh.wolff.ml@gmail.com>
---
 drivers/base/dd.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index 1bc4557a0f49..38480a01d074 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -140,7 +140,7 @@ void driver_deferred_probe_del(struct device *dev)
 	mutex_unlock(&deferred_probe_mutex);
 }
 
-static bool driver_deferred_probe_enable = false;
+static bool driver_deferred_probe_enable;
 /**
  * driver_deferred_probe_trigger() - Kick off re-probing deferred devices
  *
-- 
2.11.0

Re: [PATCH 2/2] driver core: fix statics initilisation

[Rafael J. Wysocki]

On Fri, Apr 12, 2019 at 5:48 PM Willy Wolff <willy.mh.wolff.ml@gmail.com> wrote:
>
> perl scripts/checkpatch.pl -f drivers/base/dd.c got red here:

Please note that checkpatch.pl is for new patches.  Running it against
the existing code base is questionable and so the results of that.

Re: [PATCH 2/2] driver core: fix statics initilisation

[Willy Wolff]

Thank you for your review.

I follow https://kernelnewbies.org/FirstKernelPatch to write this patch.
It's not stated that checkpatch.pl is for new patches only. Moreover,
https://kernelnewbies.org/FirstKernelPatch#Running_checkpatch.pl suggest to
run over the entire file.

Also, uninitialised static global variable are initialised to 0 by default.
Thus, initialising driver_deferred_probe_enable to false (which is 0) is
redundant.

As my knowledge, initialised global goes to .data section, and uninitialised
goes to .bss.

What does it mean for the kernel? Is this still hold?
Are performance or memory footprint of the kernel be affected?
Please, clarify my knowledge of C and kernel developement if I'm wrong.

I think checkpatch.pl should mention that information as a reminder.

Best Regards,
Willy

On Mon, Apr 15, 2019 at 10:12:43AM +0200, Rafael J. Wysocki wrote:
> On Fri, Apr 12, 2019 at 5:48 PM Willy Wolff <willy.mh.wolff.ml@gmail.com> wrote:
> >
> > perl scripts/checkpatch.pl -f drivers/base/dd.c got red here:
> 
> Please note that checkpatch.pl is for new patches.  Running it against
> the existing code base is questionable and so the results of that.

Re: [PATCH 2/2] driver core: fix statics initilisation

[Rafael J. Wysocki]

On Mon, Apr 15, 2019 at 5:29 PM Willy Wolff <willy.mh.wolff.ml@gmail.com> wrote:
>
> Thank you for your review.
>
> I follow https://kernelnewbies.org/FirstKernelPatch to write this patch.
> It's not stated that checkpatch.pl is for new patches only. Moreover,
> https://kernelnewbies.org/FirstKernelPatch#Running_checkpatch.pl suggest to
> run over the entire file.

I'm not sure about the source of that recommendation.

In any case, running it over the entire file doesn't mean that you
should or even need to make all of the warnings in that file go away.

> Also, uninitialised static global variable are initialised to 0 by default.
> Thus, initialising driver_deferred_probe_enable to false (which is 0) is
> redundant.

Yes, it is redundant, but it also is harmless AFAICS.

> As my knowledge, initialised global goes to .data section, and uninitialised
> goes to .bss.
>
> What does it mean for the kernel? Is this still hold?

No, I don't think so.

> Are performance or memory footprint of the kernel be affected?

No, they aren't.

The only difference this makes is the removal of redundant
initialization to 0, which may be regarded as a cleanup, but not as a
fix IMO.

If that's the only reason you have to change the file in question,
doing something else instead of that may be a better allocation of
your time.

Thanks!

Re: [PATCH 2/2] driver core: fix statics initilisation

[Willy Wolff]

I guess this patch will be ignored. I will drop it then.

Thanks for the discussion.

On Tue, Apr 16, 2019 at 10:50:23am +0200, Rafael J. Wysocki wrote:
> On Mon, Apr 15, 2019 at 5:29 PM Willy Wolff <willy.mh.wolff.ml@gmail.com> wrote:
> >
> > Thank you for your review.
> >
> > I follow https://kernelnewbies.org/FirstKernelPatch to write this patch.
> > It's not stated that checkpatch.pl is for new patches only. Moreover,
> > https://kernelnewbies.org/FirstKernelPatch#Running_checkpatch.pl suggest to
> > run over the entire file.
>
> I'm not sure about the source of that recommendation.
>
> In any case, running it over the entire file doesn't mean that you
> should or even need to make all of the warnings in that file go away.
>

I understand that, but it was shown as ERROR. There are some other warnings for
long lines that I skip.

> > Also, uninitialised static global variable are initialised to 0 by default.
> > Thus, initialising driver_deferred_probe_enable to false (which is 0) is
> > redundant.
>
> Yes, it is redundant, but it also is harmless AFAICS.
>
> > As my knowledge, initialised global goes to .data section, and uninitialised
> > goes to .bss.
> >
> > What does it mean for the kernel? Is this still hold?
>
> No, I don't think so.
>
> > Are performance or memory footprint of the kernel be affected?
>
> No, they aren't.
>
> The only difference this makes is the removal of redundant
> initialization to 0, which may be regarded as a cleanup, but not as a
> fix IMO.

Ok, will stated as a cleanup in the future.

>
> If that's the only reason you have to change the file in question,
> doing something else instead of that may be a better allocation of
> your time.

Thank you for your advice. I was just having my nose on it while playing with
something else, it wasn't a hide-and-seek game.

>
> Thanks!

Re: [PATCH 2/2] driver core: fix statics initilisation

[Greg Kroah-Hartman]

On Mon, Apr 15, 2019 at 04:29:01PM +0100, Willy Wolff wrote:
> Thank you for your review.
> 
> I follow https://kernelnewbies.org/FirstKernelPatch to write this patch.
> It's not stated that checkpatch.pl is for new patches only. Moreover,
> https://kernelnewbies.org/FirstKernelPatch#Running_checkpatch.pl suggest to
> run over the entire file.

FirstKernelPatch is a good thing to do in drivers/staging/, not anywhere
else in the kernel tree unless you _KNOW_ the maintainer of that
subsystem is ok to take tiny cleanup patches like this.

thanks,

greg k-h