Re: [PATCH] block/genhd.c: fix sparse warning

From: Vegard Nossum
Date: Thu Apr 16 2009 - 00:46:46 EST


2009/4/16 H Hartley Sweeten <hartleys@xxxxxxxxxxxxxxxxxxx>:
> Fix sparse warning in block/genhd.c.
>
> Â Â Â Âwarning: symbol '__mptr' shadows an earlier one
>
> Signed-off-by: H Hartley Sweeten <hsweeten@xxxxxxxxxxxxxxxxxxx>

Hi,

Just a heads up: There seems to be some sort of consensus that this
type of patch title is not a very good one. (What about "remove
variable shadowing"?)

It would also be nice to have an explanation of where the __mptr
symbol comes into play, because it doesn't even appear in the patch,
and reviewers would likely have an easier job if they knew where to
look it up.

Was this warning harmless, or was the code in fact broken?

Can we rewrite container_of() to not use an extra variable (__mptr),
or perhaps using an inline function for part of the computation?

Do we also have this problem in expressions like max(max(x, y), z)?

Thanks :-)


Vegard

>
> ---
>
> diff --git a/block/genhd.c b/block/genhd.c
> index a9ec910..ca3eedb 100644
> --- a/block/genhd.c
> +++ b/block/genhd.c
> @@ -576,8 +576,10 @@ struct gendisk *get_gendisk(dev_t devt, int
> *partno)
> Â Â Â Â Â Â Â Âstruct kobject *kobj;
>
> Â Â Â Â Â Â Â Âkobj = kobj_lookup(bdev_map, devt, partno);
> - Â Â Â Â Â Â Â if (kobj)
> - Â Â Â Â Â Â Â Â Â Â Â disk = dev_to_disk(kobj_to_dev(kobj));
> + Â Â Â Â Â Â Â if (kobj) {
> + Â Â Â Â Â Â Â Â Â Â Â struct device *dev = kobj_to_dev(kobj);
> + Â Â Â Â Â Â Â Â Â Â Â disk = dev_to_disk(dev);
> + Â Â Â Â Â Â Â }
> Â Â Â Â} else {
> Â Â Â Â Â Â Â Âstruct hd_struct *part;
>
> @@ -1010,7 +1012,7 @@ static int diskstats_show(struct seq_file *seqf,
> void *v)
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â"wsect wuse running use aveq"
> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â"\n\n");
> Â Â Â Â*/
> -
> +
> Â Â Â Âdisk_part_iter_init(&piter, gp, DISK_PITER_INCL_PART0);
> Â Â Â Âwhile ((hd = disk_part_iter_next(&piter))) {
> Â Â Â Â Â Â Â Âcpu = part_stat_lock();
> @@ -1034,7 +1036,7 @@ static int diskstats_show(struct seq_file *seqf,
> void *v)
> Â Â Â Â Â Â Â Â Â Â Â Â);
> Â Â Â Â}
> Â Â Â Âdisk_part_iter_exit(&piter);
> -
> +
> Â Â Â Âreturn 0;
> Â}
--
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/