netdev - Re: [PATCH 01/04] smc91x: pass along private data

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] [day] [month] [year] [list]

Date:	Wed, 20 Feb 2008 22:35:18 -0500 (EST)
From:	Nicolas Pitre <nico@....org>
To:	Magnus Damm <magnus.damm@...il.com>
Cc:	netdev@...r.kernel.org, lethal@...ux-sh.org,
	akpm@...ux-foundation.org
Subject: Re: [PATCH 01/04] smc91x: pass along private data

On Thu, 21 Feb 2008, Magnus Damm wrote:

> On Thu, Feb 21, 2008 at 12:42 AM, Nicolas Pitre <nico@....org> wrote:
> > On Wed, 20 Feb 2008, Magnus Damm wrote:
> >
> >  > Pass a private data pointer to macros and functions. This makes it easy
> >  > to later on make run time decisions. This patch does not change any logic.
> >  > These changes should be optimized away during compilation.
> >  >
> >  > Signed-off-by: Magnus Damm <damm@...l.co.jp>
> >  > ---
> >
> > > --- 0001/drivers/net/smc91x.c
> >  > +++ work/drivers/net/smc91x.c 2008-02-20 16:52:48.000000000 +0900
> >  > @@ -220,23 +220,23 @@ static void PRINT_PKT(u_char *buf, int l
> >  >
> >  >
> >  >  /* this enables an interrupt in the interrupt mask register */
> >  > -#define SMC_ENABLE_INT(x) do {                                               \
> >  > +#define SMC_ENABLE_INT(priv, x) do {                                 \
> >  >       unsigned char mask;                                             \
> >  > -     spin_lock_irq(&lp->lock);                                       \
> >  > -     mask = SMC_GET_INT_MASK();                                      \
> >  > +     spin_lock_irq(&priv->lock);                                     \
> >  > +     mask = SMC_GET_INT_MASK(priv);                                  \
> >
> >  Since "lp" is already used all over the place, could you simply use "lp"
> >  for the macro argument name as well instead of "priv"?  This will make
> >  the code more uniform and reduce the patch size.
> 
> I used the name "priv" instead of "lp" intentionally to make sure I
> got compile errors if I missed something. Some variables like "ioaddr"
> are today not passed as arguments to the macros but simply assumed to
> be present as local variables. I wanted to avoid using the local "lp"
> variable by mistake. So the "priv" name is actually a feature. =)

Well, you end up using the local lp variable in all cases anyway.

> I'd be happy to rewrite the patch to use "lp" though, but I have to
> confess that I don't see the point in redoing it. Anyway, please let
> me know what you prefer.

I do prefer to have lp everywhere.  But you simply have to search and 
replace "priv" with "lp" in the patch directly.  Then just applying it 
and rediffing will take care of the simplification.


Nicolas
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html