On Thu, 2004-06-24 at 09:38, Anton Altaparmakov wrote:
> On Thu, 24 Jun 2004, Anton Altaparmakov wrote:
> > On Thu, 24 Jun 2004, FabF wrote:
> >> Here's a new version:
> >> 	-macro simplification (We're calling from struct & buffer so static 
> >> fx
> >> conversion seems troublesome)
> >> 	-(p)
> >> 	-MBR patch only
> >
> > Your patch is wrong:
> >
> > diff -Naur orig/include/linux/genhd.h edited/include/linux/genhd.h
> > --- orig/include/linux/genhd.h  2004-06-16 07:18:59.000000000 +0200
> > +++ edited/include/linux/genhd.h        2004-06-24 08:02:45.151489490 +0200
> > @@ -17,6 +17,9 @@
> > #include <linux/string.h>
> > #include <linux/fs.h>
> >
> > +/*Master boot record magic numbers*/
> > +#define MSDOS_MBR(p) le16_to_cpu((u16)*(p)) == 0xaa55
> >
> > This macro is completely broken for all p with a type size other than 16 
> > bits.  E.g. if you have "char *p" then *(p) gives you a single byte which you 
> > then cast to u16 so you will never see the 0xaa55.
> >
> > If you want to do it this way you need to cast p to (u16*) first like so:
> >
> > #define MSDOS_MBR(p)	(le16_to_cpup((u16*)(p)) == 0xaa55)
> >
> > Note using _cpup() means you don't have to do the dereferencing yourself 
> > which looks much cleaner IMO.  You do still need the cast as it is missing 
> > from various macros in the kernel (I consider this a bug but this is 
> > orthogonal to this patch).
> 
> While I am at it, the above macro is even further optimized by moving the 
> endianness conversion to the constant so it is applied at compile time 
> rather than run time like so:
> 
> #define MSDOS_MBR(p)	((*(u16*)(p)) == __constant_cpu_to_le16(0xaa55)
Thanks Anton :) Applied.It worked before but in semi-evaluation.

Regards,
FabF

> 
> Best regards,
> 
>  	Anton