linux-kernel - Re: [PATCH -mm 5/9] netconsole: Introduce dev

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Pine.LNX.4.64.0707051501190.15925@cselinux1.cse.iitk.ac.in>
Date:	Thu, 5 Jul 2007 15:06:01 +0530 (IST)
From:	Satyam Sharma <ssatyam@....iitk.ac.in>
To:	Joel Becker <Joel.Becker@...cle.com>
cc:	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Keiichi Kii <k-keiichi@...jp.nec.com>,
	Netdev <netdev@...r.kernel.org>, Matt Mackall <mpm@...enic.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	David Miller <davem@...emloft.net>
Subject: Re: [PATCH -mm 5/9] netconsole: Introduce dev_status member

On Wed, 4 Jul 2007, Joel Becker wrote:
> On Wed, Jul 04, 2007 at 04:38:04PM +0530, Satyam Sharma wrote:
> > -	if (!(event == NETDEV_CHANGEADDR || event == NETDEV_CHANGENAME))
> > -		goto done;
> > +	if (!(event == NETDEV_UP || event == NETDEV_DOWN ||
> > +	      event == NETDEV_CHANGEADDR || event == NETDEV_CHANGENAME))
> > +	      	goto done;
> >
> >  	if (nt->np.dev == dev) {
> >  		switch (event) {
> 
> 	It's a small nit, but isn't the large if() just the degenerate
> default case of the switch()?  Why have it at all?  

Yes, the large if() is redundant in this particular patch.

But in further patches, the switch() is enclosed inside a 
spin_lock_irqsave() and list_for_each_entry(target_list) and so the
large if() upfront saves us from unnecessarily disabling interrupts
and iterating through the entire target_list in the case that it is
a notification for an event that we don't care about.

So I'll remove this if() from this patch and introduce it in the
later one itself, which is where it starts making some sense, anyway.

Satyam
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/