Messages in this thread | | | Date | Fri, 8 Oct 2004 12:12:39 -0700 | From | Andrew Morton <> | Subject | Re: [patch 1/1] dm: fix printk warnings about whether %lu/%Lu is right for sector_t |
| |
(cc'ed Neil)
blaisorblade_spam@yahoo.it wrote: > > > The Device Manager code barfs when sector_t is 64bit wide (i.e. an u64); > this can happen with CONFIG_LBD on i386, too, but sector_t is usually a long. > And region_t, chunk_t are typedefs for sector_t. > > The problem is this code in drivers/md/dm.h (wouldn't we better fix the > FIXME?): > > /* > * FIXME: I think this should be with the definition of sector_t > * in types.h. > */ > #ifdef CONFIG_LBD > #define SECTOR_FORMAT "%Lu" > #else > #define SECTOR_FORMAT "%lu" > #endif > > Btw, x86_64 does not need to define sector_t on its own. > If there is any _good_ reason for that, the fix becomes adding a > #define SECTOR_FORMAT "%Lu" > in fact, gcc tries to be smart for warnings to ensure portability; so, even > when sizeof(long) == sizeof(long long), "%ld" and "%Ld" are different, for the > warnings). > > Sample warnings (from both 2.6.8.1 and 2.6.9-rc2): > drivers/md/dm-raid1.c: In function `get_mirror': > drivers/md/dm-raid1.c:930: warning: long unsigned int format, sector_t arg (arg 3) > drivers/md/dm-raid1.c: In function `mirror_status': > drivers/md/dm-raid1.c:1200: warning: long unsigned int format, region_t arg (arg 4) > drivers/md/dm-raid1.c:1200: warning: long unsigned int format, region_t arg (arg 5) > drivers/md/dm-raid1.c:1206: warning: long unsigned int format, sector_t arg (arg 5) > drivers/md/dm-raid1.c:1212: warning: long unsigned int format, sector_t arg (arg 5) >
I would prefer that SECTOR_FORMAT be removed altogether.
The industry-standard way of printing a sector_t is:
printk("%llu", (unsigned long long)sector);
Or %Ld or %llo or whatever. The key point is that the format string should specify long long and the argument should be typecast to long long.
The sector_t should not be typecast to u64, btw. We must never typecast printk arguments to u64 because u64 is long long on some architectures and long on others.
> diff -puN drivers/md/dm.h~fix-dm-warnings drivers/md/dm.h > --- linux-2.6.9-current/drivers/md/dm.h~fix-dm-warnings 2004-10-08 16:39:05.888125592 +0200 > +++ linux-2.6.9-current-paolo/drivers/md/dm.h 2004-10-08 16:39:05.942117384 +0200 > @@ -22,16 +22,6 @@ > #define DMEMIT(x...) sz += ((sz >= maxlen) ? \ > 0 : scnprintf(result + sz, maxlen - sz, x)) > > -/* > - * FIXME: I think this should be with the definition of sector_t > - * in types.h. > - */ > -#ifdef CONFIG_LBD > -#define SECTOR_FORMAT "%Lu" > -#else > -#define SECTOR_FORMAT "%lu" > -#endif > - > #define SECTOR_SHIFT 9 > > /* > diff -puN include/asm-h8300/types.h~fix-dm-warnings include/asm-h8300/types.h > --- linux-2.6.9-current/include/asm-h8300/types.h~fix-dm-warnings 2004-10-08 16:39:05.902123464 +0200 > +++ linux-2.6.9-current-paolo/include/asm-h8300/types.h 2004-10-08 16:39:05.959114800 +0200 > @@ -56,6 +56,7 @@ typedef unsigned long long u64; > typedef u32 dma_addr_t; > > #define HAVE_SECTOR_T > +#define SECTOR_FORMAT "%Lu" > typedef u64 sector_t; > > typedef unsigned int kmem_bufctl_t; > diff -puN include/asm-i386/types.h~fix-dm-warnings include/asm-i386/types.h > --- linux-2.6.9-current/include/asm-i386/types.h~fix-dm-warnings 2004-10-08 16:39:05.914121640 +0200 > +++ linux-2.6.9-current-paolo/include/asm-i386/types.h 2004-10-08 16:39:05.959114800 +0200 > @@ -60,6 +60,7 @@ typedef u64 dma64_addr_t; > > #ifdef CONFIG_LBD > typedef u64 sector_t; > +#define SECTOR_FORMAT "%Lu" > #define HAVE_SECTOR_T > #endif > > diff -puN include/asm-mips/types.h~fix-dm-warnings include/asm-mips/types.h > --- linux-2.6.9-current/include/asm-mips/types.h~fix-dm-warnings 2004-10-08 16:39:05.915121488 +0200 > +++ linux-2.6.9-current-paolo/include/asm-mips/types.h 2004-10-08 16:39:05.980111608 +0200 > @@ -96,6 +96,7 @@ typedef unsigned long phys_t; > > #ifdef CONFIG_LBD > typedef u64 sector_t; > +#define SECTOR_FORMAT "%Lu" > #define HAVE_SECTOR_T > #endif > > diff -puN include/asm-ppc/types.h~fix-dm-warnings include/asm-ppc/types.h > --- linux-2.6.9-current/include/asm-ppc/types.h~fix-dm-warnings 2004-10-08 16:39:05.916121336 +0200 > +++ linux-2.6.9-current-paolo/include/asm-ppc/types.h 2004-10-08 16:39:05.980111608 +0200 > @@ -59,6 +59,7 @@ typedef u64 dma64_addr_t; > > #ifdef CONFIG_LBD > typedef u64 sector_t; > +#define SECTOR_FORMAT "%Lu" > #define HAVE_SECTOR_T > #endif > > diff -puN include/asm-s390/types.h~fix-dm-warnings include/asm-s390/types.h > --- linux-2.6.9-current/include/asm-s390/types.h~fix-dm-warnings 2004-10-08 16:39:05.917121184 +0200 > +++ linux-2.6.9-current-paolo/include/asm-s390/types.h 2004-10-08 16:39:05.981111456 +0200 > @@ -92,6 +92,7 @@ typedef union { > > #ifdef CONFIG_LBD > typedef u64 sector_t; > +#define SECTOR_FORMAT "%Lu" > #define HAVE_SECTOR_T > #endif > > diff -puN include/asm-sh/types.h~fix-dm-warnings include/asm-sh/types.h > --- linux-2.6.9-current/include/asm-sh/types.h~fix-dm-warnings 2004-10-08 16:39:05.920120728 +0200 > +++ linux-2.6.9-current-paolo/include/asm-sh/types.h 2004-10-08 16:39:05.981111456 +0200 > @@ -55,6 +55,7 @@ typedef u32 dma_addr_t; > > #ifdef CONFIG_LBD > typedef u64 sector_t; > +#define SECTOR_FORMAT "%Lu" > #define HAVE_SECTOR_T > #endif > > diff -puN include/asm-x86_64/types.h~fix-dm-warnings include/asm-x86_64/types.h > --- linux-2.6.9-current/include/asm-x86_64/types.h~fix-dm-warnings 2004-10-08 16:39:05.921120576 +0200 > +++ linux-2.6.9-current-paolo/include/asm-x86_64/types.h 2004-10-08 16:39:05.981111456 +0200 > @@ -48,9 +48,6 @@ typedef unsigned long long u64; > typedef u64 dma64_addr_t; > typedef u64 dma_addr_t; > > -typedef u64 sector_t; > -#define HAVE_SECTOR_T > - > typedef unsigned short kmem_bufctl_t; > > #endif /* __ASSEMBLY__ */ > diff -puN include/linux/types.h~fix-dm-warnings include/linux/types.h > --- linux-2.6.9-current/include/linux/types.h~fix-dm-warnings 2004-10-08 16:39:05.939117840 +0200 > +++ linux-2.6.9-current-paolo/include/linux/types.h 2004-10-08 16:39:05.959114800 +0200 > @@ -130,6 +130,7 @@ typedef __s64 int64_t; > */ > #ifndef HAVE_SECTOR_T > typedef unsigned long sector_t; > +#define SECTOR_FORMAT "%lu" > #endif > > /* > _ > - > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |