Re: + shut-up-warnings-in-ipc-shmc.patch added to -mm tree

[Matt Mackall <mpm@selenic.com>]

[thanks for the cc]

On Thu, Nov 24, 2005 at 12:47:15PM +0000, Hugh Dickins wrote:
> On Tue, 22 Nov 2005 akpm@osdl.org wrote:
> > 
> > The patch titled
> > 
> >      Shut up warnings in ipc/shm.c
> > 
> > has been added to the -mm tree.  Its filename is
> > 
> >      shut-up-warnings-in-ipc-shmc.patch
> > 
> > 
> > From: Russell King <rmk@arm.linux.org.uk>
> > 
> > Fix two warnings in ipc/shm.c
> > 
> > ipc/shm.c:122: warning: statement with no effect
> > ipc/shm.c:560: warning: statement with no effect
> > 
> > by converting the macros to empty inline functions.  For safety, let's do
> > all three.  This also has the advantage that typechecking gets performed
> > even without CONFIG_SHMEM enabled.
> 
> Sorry to be a nuisance, but I'm a little resistant to this patch.
> Which version(s) of the compiler gives that warning?
> Aren't there 5000 other such stub #defines which should also be changed?
> Or is the problem the rather complex "({0;})" - should that be "0"?
> It seems such clutter to use 6 lines of inline function for each of these.
> Nice try, but I don't buy the typechecking advantage in this case!

Unfortunately Russell didn't tell us which function caused the error
and I can't seem to find a tree that matches his line numbering.
But it looks like it's shm_unlock.

The current ({0;}) seems wrong to me. I'd expect that expression to be
void. Hmm, looks like I'm wrong. It's quite ugly, not to mention confusing.

Andrew introduced it in a patch called "[PATCH] Fix shmem.c
stubs" that did this:

-#define shmem_lock(a, b) /* always in memory, no need to lock */
+#define shmem_lock(a, b, c) ({0;}) /* always in memory, no need to lock */

(shmem_lock changed from void to int a few days before this with
"rlimit-based mlocks for unprivileged users")

I didn't get compile warnings when I introduced tiny-shmem in 2004
(or, for that matter, when I wrote it in 2003) but I do seem to be
getting them now with gcc 4 -W.

So apparently gcc has gotten more picky about such things.

If we're going to start converting such things, I'd almost rather do
something like:

kernel.h:
static inline void empty_void(void) {}
static inline void empty_int(void) { return 0; }
...

mm.h:
#define shm_lock(a, b) empty_int()

The typechecking is nice in theory, but in practice I don't think it
really makes a difference for stubbing things out.

-- 
Mathematics is the supreme nostalgia of our time.