Re: [patch 5/9] x86_64: arch_pick_mmap_layout() fixlet

[Josh Triplett <josht@linux.vnet.ibm.com>]

On Fri, 2007-07-06 at 10:21 -0700, Linus Torvalds wrote:
> This would be a sparse bug, I think.
> 
> Returning a void-valued expression in a void-valued function is 
> proper C99 (and gcc has allowed it for longer). And it often makes for 
> nicer-looking code, with no real downsides.
> 
> This seems to be a new bug introduced by Al Viro in sparse: -Wall should 
> _not_ enable -Wreturn-void, I think (and I don't think Al meant it to: his 
> commit message says "Conditional on -Wreturn-void", and he probably didn't 
> think about the fact that -Wall will force them all on.
> 
> Josh, Al and sparse mailing list added to participants list, and patch 
> dropped. I think -Wreturn-void is fine, but it just shouldn't be part of 
> -Wall, and thus shouldn't trigger for the kernl.

The kernel does not seem to use sparse -Wall by default.  Did the patch
submitter use CF=-Wall or CF=-Wreturn-void, or otherwise change the set
of warnings emitted by Sparse?

I do think the sparse warning logic needs some work.  -Wall logically
should mean "all warnings", but gcc historically uses it to mean "some
reasonable set of warnings".  However, since Sparse exists solely to
give warnings, it defaults to "some reasonable set of warnings", and
uses -Wall for "all warnings".  Perhaps for gcc compatibility, that
should change.  I'd love to hear suggestions for how Sparse should
behave.  A few options:

      * Leave the current behavior, and tell people annoyed by Sparse
        warnings they don't care about that they should either avoid
        -Wall or use -Wall -Wno-warning-i-do-not-care-about.
      * Ignore -Wall since we already give a reasonable default set of
        warnings, and possibly introduce a -Weverything or similar that
        really does mean "turn on every Sparse warning".
      * Change -Wall to mean "turn on some additional warnings, but not
        all of them".  We then have to decide which warnings -Wall
        should enable, and again we might want to add a -Weverything.
      * Turn off all warnings by default unless you give -Wall.  This
        seems entirely broken, given the purpose of Sparse.

> On Fri, 6 Jul 2007, akpm@linux-foundation.org wrote:
> > From: Alexey Dobriyan <adobriyan@sw.ru>
> > 
> > sparse now warns about
> > arch/x86_64/mm/mmap.c:15:3: warning: returning void-valued expression

- Josh Triplett