Re: [PATCH] drivers/net/depca.c tidyup

Re: [PATCH] drivers/net/depca.c tidyup

[Peter Denison]

On Mon, 6 May 2002, Dave Jones wrote:

> On Mon, May 06, 2002 at 06:17:00PM +0100, Peter Denison wrote:
>  > The ALIGN macro is defined centrally - remove the local version
> 
> They aren't functionally equivalent though are they?

Eek. I thought they were.

-#define ALIGN4      ((u_long)4 - 1)       /* 1 longword align */
-#define ALIGN8      ((u_long)8 - 1)       /* 2 longword (quadword) align */
-#define ALIGN         ALIGN8              /* Keep the LANCE happy... */

-	offset = (offset + ALIGN) & ~ALIGN;
+	offset = ALIGN(offset, 8);

And from include/linux/cache.h:

#define ALIGN(x,a) (((x)+(a)-1)&~((a)-1))

So, we're replacing (offset + 8 - 1) & ~(8-1) = (offset + 7) & ~7
with (((offset)+(8)-1)&~((8)-1)) = ((offset+7)&~7)

I think they're the same, aren't they, except for the promotion to u_long, 
which is probably bogus anyway?

-- 
Peter Denison <peterd at marshadder dot uklinux dot net>
Please note that my address is changing from <peterd at pnd-pc dot demon.co.uk>

Re: [PATCH] drivers/net/depca.c tidyup

[Dave Jones]

On Mon, May 06, 2002 at 08:01:01PM +0100, Peter Denison wrote:

 > -	offset = (offset + ALIGN) & ~ALIGN;
 > +	offset = ALIGN(offset, 8);
 > 
 > And from include/linux/cache.h:
 > 
 > #define ALIGN(x,a) (((x)+(a)-1)&~((a)-1))
 > 
 > So, we're replacing (offset + 8 - 1) & ~(8-1) = (offset + 7) & ~7
 > with (((offset)+(8)-1)&~((8)-1)) = ((offset+7)&~7)

Argh, I was looking at the definition in linkage.h
Cursed ctags.

-- 
| Dave Jones.        http://www.codemonkey.org.uk
| SuSE Labs