Re: [PATCH] Fix Warnings from net/netlink/genetlink.c

Stephen Rothwell
2009-08-12T03:50:40Z

[-- Attachment #1: Type: text/plain, Size: 2432 bytes --]

Hi all,

On Tue, 11 Aug 2009 20:24:59 -0700 Stephen Hemminger <shemminger@vyatta.com> wrote:
>
> > > @@ -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');
> >
> > this looks fishy. How does gcc thinks this variable is uninitialized. If
> > I look at the code in Linus' tree, I don't see it.
>
> Agreed, and the line numbers are off.

In the -next tree, it looks like this:

int genl_register_mc_group(struct genl_family *family,
struct genl_multicast_group *grp)
{
int id;
unsigned long *new_groups;
int err;

BUG_ON(grp->name[0] == '\0');

genl_lock();

/* special-case our own group */
if (grp == &notify_grp)
id = GENL_ID_CTRL;
else
id = find_first_zero_bit(mc_groups,
mc_groups_longs * BITS_PER_LONG);

if (id >= mc_groups_longs * BITS_PER_LONG) {
size_t nlen = (mc_groups_longs + 1) * sizeof(unsigned long);

if (mc_groups == &mc_group_start) {
new_groups = kzalloc(nlen, GFP_KERNEL);
if (!new_groups) {
err = -ENOMEM;
goto out;
}
mc_groups = new_groups;
*mc_groups = mc_group_start;
} else {
new_groups = krealloc(mc_groups, nlen, GFP_KERNEL);
if (!new_groups) {
err = -ENOMEM;
goto out;
}
mc_groups = new_groups;
mc_groups[mc_groups_longs] = 0;
}
mc_groups_longs++;
}

if (family->netnsok) {
struct net *net;

rcu_read_lock();
for_each_net_rcu(net) {
err = netlink_change_ngroups(net->genl_sock,
mc_groups_longs * BITS_PER_LONG);
if (err) {
rcu_read_unlock();
goto out;
}
}
rcu_read_unlock();
} else {
err = netlink_change_ngroups(init_net.genl_sock,
mc_groups_longs * BITS_PER_LONG);
if (err)
goto out;
}

grp->id = id;
set_bit(id, mc_groups);
list_add_tail(&grp->list, &family->mcast_groups);
grp->family = family;

genl_ctrl_event(CTRL_CMD_NEWMCAST_GRP, grp);
out:
genl_unlock();
return err;
}

so if family->netnsok is true and the for_each_net_rcu loop executes 0
times, err will not be set ... Now, that may not be logically possible,
but I can't tell that from this code.

--
Cheers,
Stephen Rothwell sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 197 bytes --]