[PATCH] netlink: silence compiler warning

[PATCH] netlink: silence compiler warning

[Brian Haley]

  CC      net/netlink/genetlink.o
net/netlink/genetlink.c: In function ‘genl_register_mc_group’:
net/netlink/genetlink.c:139: warning: ‘err’ may be used uninitialized in this function

From following the code 'err' is initialized, but set it to zero to
silence the warning.

Signed-off-by: Brian Haley <brian.haley@hp.com>
---

diff --git a/net/netlink/genetlink.c b/net/netlink/genetlink.c
index 575c643..66f6ba0 100644
--- a/net/netlink/genetlink.c
+++ b/net/netlink/genetlink.c
@@ -136,7 +136,7 @@ int genl_register_mc_group(struct genl_family *family,
 {
 	int id;
 	unsigned long *new_groups;
-	int err;
+	int err = 0;
 
 	BUG_ON(grp->name[0] == '\0');

Re: [PATCH] netlink: silence compiler warning

[Marcel Holtmann]

Hi Brian,

> CC      net/netlink/genetlink.o
> net/netlink/genetlink.c: In function ‘genl_register_mc_group’:
> net/netlink/genetlink.c:139: warning: ‘err’ may be used uninitialized in this function
> 
> From following the code 'err' is initialized, but set it to zero to
> silence the warning.
> 
> Signed-off-by: Brian Haley <brian.haley@hp.com>
> ---
> 
> diff --git a/net/netlink/genetlink.c b/net/netlink/genetlink.c
> index 575c643..66f6ba0 100644
> --- a/net/netlink/genetlink.c
> +++ b/net/netlink/genetlink.c
> @@ -136,7 +136,7 @@ int genl_register_mc_group(struct genl_family *family,
>  {
>  	int id;
>  	unsigned long *new_groups;
> -	int err;
> +	int err = 0;
>  
>  	BUG_ON(grp->name[0] == '\0');

can we please add the err = -E... where it actually is needed and not
stupidly go ahead and silence compiler warnings with err = 0. This has
been posted before.

Regards

Marcel

Re: [PATCH] netlink: silence compiler warning

[Brian Haley]

Hi Marcel,

Marcel Holtmann wrote:
> can we please add the err = -E... where it actually is needed and not
> stupidly go ahead and silence compiler warnings with err = 0. This has
> been posted before.

Sorry, I don't remember it being posted before.  If you look at the code
though, err is correctly initialized, gcc just can't figure it out.  The
choices I see are either what I originally posted, using uninitialized_var(err),
or the patch below.  It doesn't matter to me.

-Brian


diff --git a/net/netlink/genetlink.c b/net/netlink/genetlink.c
index 66f6ba0..8741036 100644
--- a/net/netlink/genetlink.c
+++ b/net/netlink/genetlink.c
@@ -177,6 +177,7 @@ int genl_register_mc_group(struct genl_family *family,
 		struct net *net;
 
 		rcu_read_lock();
+		err = 0;
 		for_each_net_rcu(net) {
 			err = netlink_change_ngroups(net->genl_sock,
 					mc_groups_longs * BITS_PER_LONG);

Re: [PATCH] netlink: silence compiler warning

[David Miller]

From: Brian Haley <brian.haley@hp.com>
Date: Fri, 04 Sep 2009 21:36:06 -0400

> Hi Marcel,
> 
> Marcel Holtmann wrote:
>> can we please add the err = -E... where it actually is needed and not
>> stupidly go ahead and silence compiler warnings with err = 0. This has
>> been posted before.
> 
> Sorry, I don't remember it being posted before.  If you look at the code
> though, err is correctly initialized, gcc just can't figure it out.  The
> choices I see are either what I originally posted, using uninitialized_var(err),
> or the patch below.  It doesn't matter to me.

uninitialized_var() would be absolutely wrong here, as then we'd
return garbage if such a path were actually possible.

Your original patch was fine and I'm going to apply it, thanks.

Re: [PATCH] netlink: silence compiler warning

[Jarek Poplawski]

David Miller wrote, On 09/05/2009 05:33 AM:

> From: Brian Haley <brian.haley@hp.com>
> Date: Fri, 04 Sep 2009 21:36:06 -0400
> 
>> Hi Marcel,
>>
>> Marcel Holtmann wrote:
>>> can we please add the err = -E... where it actually is needed and not
>>> stupidly go ahead and silence compiler warnings with err = 0. This has
>>> been posted before.
>> Sorry, I don't remember it being posted before.  If you look at the code
>> though, err is correctly initialized, gcc just can't figure it out.  The
>> choices I see are either what I originally posted, using uninitialized_var(err),
>> or the patch below.  It doesn't matter to me.
> 
> uninitialized_var() would be absolutely wrong here, as then we'd
> return garbage if such a path were actually possible.


If the main "thesis" of the patch is:

> From following the code 'err' is initialized, but set it to zero to
> silence the warning.

"we" should better be sure it "is initialized", so considering: "if such
a path were actually possible" "would be absolutely wrong here"...

Here is a link to the message which proved something else was possible
at some moment in -next (I didn't check the current code yet):

From: Stephen Rothwell <sfr@canb.auug.org.au>
Subject: Re: [PATCH] Fix Warnings from net/netlink/genetlink.c
Date: Wed, 12 Aug 2009 13:50:31 +1000

Archived-At: http://permalink.gmane.org/gmane.linux.kernel.next/8786

Jarek P.

Re: [PATCH] netlink: silence compiler warning

[David Miller]

From: Jarek Poplawski <jarkao2@gmail.com>
Date: Sat, 05 Sep 2009 09:52:40 +0200

> http://permalink.gmane.org/gmane.linux.kernel.next/8786

Right, and that was my thinking.  So setting it to zero to
begin with is the right thing to do.

Re: [PATCH] netlink: silence compiler warning

[David Miller]

From: Marcel Holtmann <marcel@holtmann.org>
Date: Sat, 05 Sep 2009 03:08:01 +0200

> can we please add the err = -E... where it actually is needed and not
> stupidly go ahead and silence compiler warnings with err = 0. This has
> been posted before.

Ummm, no.  Actually Brian's patch is correct.

I can't even prove that there are no paths where err won't
be initialized properly to something.

And if such an occurance happens, returning "0" is absolutely
the right thing to do.

And that's what Brian's patch does.