Messages in this thread | | | Subject | RE: [PATCH] block/genhd.c: fix sparse warning | Date | Thu, 16 Apr 2009 14:03:39 -0400 | From | "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@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/
| |