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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <19f34abd0904152146l3db9961bt3b52f224abc8d960@mail.gmail.com>
Date:	Thu, 16 Apr 2009 06:46:32 +0200
From:	Vegard Nossum <vegard.nossum@...il.com>
To:	H Hartley Sweeten <hartleys@...ionengravers.com>
Cc:	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] block/genhd.c: fix sparse warning

2009/4/16 H Hartley Sweeten <hartleys@...ionengravers.com>:
> Fix sparse warning in block/genhd.c.
>
>        warning: symbol '__mptr' shadows an earlier one
>
> Signed-off-by: H Hartley Sweeten <hsweeten@...ionengravers.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;
>  }
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/