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: <BD79186B4FD85F4B8E60E381CAEE1909015F8F6D@mi8nycmail19.Mi8.com>
Date:	Thu, 16 Apr 2009 14:03:39 -0400
From:	"H Hartley Sweeten" <hartleys@...ionengravers.com>
To:	"Vegard Nossum" <vegard.nossum@...il.com>
Cc:	<linux-kernel@...r.kernel.org>
Subject: RE: [PATCH] block/genhd.c: fix sparse warning

On Wednesday, April 15, 2009 9:47 PM, Vegard Nossum wrote:
>> 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"?)

Sorry, I wasn't aware of that. I just started using sparse and was
cleaning up some warnings in the arch/arm/mach-ep93xx branch. I
noticed this warning and it seemed like a simple fix.

> 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.

The only __mptr symbol in the source is in the container_of() macro
in include/linux/kernel.h:

	#define container_of(ptr, type, member) ({			\
		const typeof( ((type *)0)->member ) *__mptr = (ptr);	\
		(type *)( (char *)__mptr - offsetof(type,member) );})

The sparse warning shows up when a macro expansion ends up with
something like:

	type1 val = container_of(container_of(ptr2, type2, member2),
						type1, member1);

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

Should be harmless, the scope of __mptr should only be in the macro.

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

Maybe something like:

	#define container_of(ptr, type, member) ({			\
		const typeof( ((type *)0)->member ) *__m_##ptr = (ptr);	\
		(type *)( (char *)__m_##ptr - offsetof(type,member) );})

I don't know if that actually works. ;-)


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

That should generate the same sparse warning since each max() has a
couple of local variables (_min1 and _min2).

> Thanks :-)

Not a problem. Just trying to help!

Regards,
Hartley
--
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/