lkml.org 
[lkml]   [2009]   [Apr]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRE: [PATCH] block/genhd.c: fix sparse warning
Date
From
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@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"?)

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

\
 
 \ /
  Last update: 2009-04-16 20:07    [from the cache]
©2003-2011 Jasper Spaans