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

[Vegard Nossum <vegard.nossum@gmail.com>]

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

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;
>  }