Re: [patch 1/1] dm: fix printk warnings about whether %lu/%Lu isright for sector_t

[Anton Altaparmakov]

On Fri, 8 Oct 2004, Andrew Morton wrote:
> blaisorblade_spam@xxxxxxxx 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.

Actually %Ld is completely wrong.  I know in the kernel it makes no 
difference but people see it in the kernel and then go off an use it in 
userspace and it generates junk output on at least some architectures.  
This is because %L means "long double (floating point)" not "long long 
integer" and when you stuff an integer into it it goes wrong (on some 
architectures)...  From the printf(3) man page:

ll     (ell-ell).   A  following integer conversion corre­
       sponds to a long long int or unsigned long long int
       argument,  or  a following n conversion corresponds
       to a pointer to a long long int argument.

L      A following a, A, e, E, f, F, g,  or  G  conversion
       corresponds to a long double argument.  (C99 allows
       %LF, but SUSv2 does not.)

Best regards,

	Anton
-- 
Anton Altaparmakov <aia21 at cam.ac.uk> (replace at with @)
Unix Support, Computing Service, University of Cambridge, CB2 3QH, UK
Linux NTFS maintainer / IRC: #ntfs on irc.freenode.net
WWW: http://linux-ntfs.sf.net/ & http://www-stu.christs.cam.ac.uk/~aia21/