Re: [PATCH 2/3] mg_disk: fix CONFIG_LBD=y warning

[Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>]

On Tuesday 28 April 2009 20:09:54 Jeff Garzik wrote:
> Bartlomiej Zolnierkiewicz wrote:
> > On Tuesday 28 April 2009 17:19:34 Mark Lord wrote:
> >> Tejun Heo wrote:
> >>> From: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
> >>>
> >>> drivers/block/mg_disk.c: In function ‘mg_dump_status’:
> >>> drivers/block/mg_disk.c:265: warning: format ‘%ld’ expects type ‘long int’, but
> >>> argument 2 has type ‘sector_t’
> >>>
> >>> [ Impact: kill build warning ]
> >>>
> >>> Cc: unsik Kim <donari75@gmail.com>
> >>> Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
> >>> Signed-off-by: Tejun Heo <tj@kernel.org>
> >>> ---
> >>>  drivers/block/mg_disk.c |    2 +-
> >>>  1 files changed, 1 insertions(+), 1 deletions(-)
> >>>
> >>> diff --git a/drivers/block/mg_disk.c b/drivers/block/mg_disk.c
> >>> index d3e72ad..f389835 100644
> >>> --- a/drivers/block/mg_disk.c
> >>> +++ b/drivers/block/mg_disk.c
> >>> @@ -79,7 +79,7 @@ static void mg_dump_status(const char *msg, unsigned int stat,
> >>>  			if (host->breq) {
> >>>  				req = elv_next_request(host->breq);
> >>>  				if (req)
> >>> -					printk(", sector=%ld", req->sector);
> >>> +					printk(", sector=%u", (u32)req->sector);
> >> ..
> >>
> >> Eh?  Shouldn't that be fixed the other way around, like this:
> >>
> >>   +					printk(", sector=%llu", (u64)req->sector);
> >>
> >> This way, it will still give correct data when sector_t is a u64.
> > 
> > shouldn't matter, req->sector is never > u32 for mg_disk
> 
> It never matters... until the code gets copied elsewhere.  IMO wrong 
> code should never be kept -- "impossible to hit" just means it is low 
> priority :)

Well, if you feel strongly about it feel free to replace my patch with
your improved version.  You may also fix hd.c (from which the above code
has been copied) while at it.