Skip to contentSkip to conversation

netlink: silence compiler warning

PATCH In review Brian Haley 4 participants 7 messages Sep 5, 2009

Subsystems (2)
Networking Network drivers
lore
Share & tools
lore

Brian Haley

author
#0 lore raw

PATCH 1/1 netlink: silence compiler warning

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 <[email protected]>

---

net/netlink/genetlink.c +1 -1
index 575c643..66f6ba0 100644
@@ -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');
#1 lore raw

Hi Brian,

Quoted 15 lines

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 <[email protected]>

---

net/netlink/genetlink.c +0 -0
index 575c643..66f6ba0 100644

net/netlink/genetlink.c int genl_register_mc_group(struct genl_family *family, · line 141

Quoted 7 lines
net/netlink/genetlink.c +1 -1
@@ -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

author
#2 lore raw

Hi Marcel,

Quoted 3 lines · Marcel Holtmann

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

net/netlink/genetlink.c +1 -0
index 66f6ba0..8741036 100644
@@ -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);
maintainer
#3 lore raw

From: Brian Haley <[email protected]>
Date: Fri, 04 Sep 2009 21:36:06 -0400

Quoted 11 lines

Hi Marcel,

Quoted 3 lines · Marcel Holtmann

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.

#4 lore raw

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

Quoted 9 lines

From: Brian Haley <[email protected]>
Date: Fri, 04 Sep 2009 21:36:06 -0400

Quoted 6 lines

Hi Marcel,

Quoted 3 lines · Marcel Holtmann

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.

Quoted 7 lines
Quoted 4 lines

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:

Quoted 2 lines

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 <[email protected]>
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.

maintainer
#6 lore raw
In reply to Marcel Holtmann

From: Marcel Holtmann <[email protected]>
Date: Sat, 05 Sep 2009 03:08:01 +0200

Quoted 3 lines

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.