public inbox for linux-next@vger.kernel.org 
 help / color / mirror / Atom feed

From: Stephen Rothwell <sfr@canb•auug.org.au>
To: Stephen Hemminger <shemminger@vyatta•com>
Cc: Marcel Holtmann <marcel@holtmann•org>,
	vibi_sreenivasan@cms•com, netdev <netdev@vger•kernel.org>,
	Johannes Berg <johannes@sipsolutions•net>,
	Thomas Graf <tgraf@suug•ch>,
	linux-next <linux-next@vger•kernel.org>
Subject: Re: [PATCH] Fix Warnings from net/netlink/genetlink.c
Date: Wed, 12 Aug 2009 13:50:31 +1000	[thread overview]
Message-ID: <20090812135031.a38be3f6.sfr@canb.auug.org.au> (raw)
In-Reply-To: <20090811202459.7f98e496@nehalam>

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

next prev parent reply	other threads:[~2009-08-12  3:50 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-08-11 10:07 [PATCH] Fix Warnings from net/netlink/genetlink.c vibi sreenivasan
2009-08-11 23:57 ` Marcel Holtmann
2009-08-12  3:24   ` Stephen Hemminger
2009-08-12  3:50     ` Stephen Rothwell [this message]
2009-08-12  4:03       ` Marcel Holtmann
     [not found] <1249975630.2400.7.camel@HunTEr>
2009-08-11  7:36 ` Johannes Berg
2009-08-11  9:15   ` vibi sreenivasan
2009-08-11  9:11     ` Johannes Berg

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20090812135031.a38be3f6.sfr@canb.auug.org.au \
    --to=sfr@canb$(echo .)auug.org.au \
    --cc=johannes@sipsolutions$(echo .)net \
    --cc=linux-next@vger$(echo .)kernel.org \
    --cc=marcel@holtmann$(echo .)org \
    --cc=netdev@vger$(echo .)kernel.org \
    --cc=shemminger@vyatta$(echo .)com \
    --cc=tgraf@suug$(echo .)ch \
    --cc=vibi_sreenivasan@cms$(echo .)com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox