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

[H Hartley Sweeten]

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@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"?)

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@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/