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

Powered by blists - more mailing lists