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

From: Matt Mackall (mpm_at_selenic.com)
Date: 11/24/05

  • Next message: Muli Ben-Yehuda: "Re: + shut-up-warnings-in-ipc-shmc.patch added to -mm tree" 
    Date:	Thu, 24 Nov 2005 08:00:12 -0800
To: Hugh Dickins <hugh@veritas.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.
-
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/
  • Next message: Muli Ben-Yehuda: "Re: + shut-up-warnings-in-ipc-shmc.patch added to -mm tree"