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

[Bartlomiej Zolnierkiewicz]

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@xxxxxxxxx>
> >>>
> >>> 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@xxxxxxxxx>
> >>> Signed-off-by: Bartlomiej Zolnierkiewicz <bzolnier@xxxxxxxxx>
> >>> Signed-off-by: Tejun Heo <tj@xxxxxxxxxx>
> >>> ---
> >>>  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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/