Re: [PATCH] init: Properly placing noinline keyword.

[Andrew Morton]

On Fri, 17 Oct 2008 18:31:23 +0100
Am__rico Wang <xiyou.wangcong@xxxxxxxxx> wrote:

> On Fri, Oct 17, 2008 at 09:10:07PM +0600, Rakib Mullick wrote:
> >On 10/17/08, Am__rico Wang <xiyou.wangcong@xxxxxxxxx> wrote:
> >> On Fri, Oct 17, 2008 at 08:17:33PM +0600, Rakib Mullick wrote:
> >>  >On 10/17/08, Alexey Dobriyan <adobriyan@xxxxxxxxx> wrote:
> >>  >> On Fri, Oct 17, 2008 at 07:05:32PM +0600, Rakib Mullick wrote:
> >>  >>  > Here, noinline keyword should be placed between storage class and type.
> >>  >>
> >>  >>
> >>  >> Why?
> >>  >Because, scripts/checkpatch.pl warned with following warning:
> >>  >     ERROR: inline keyword should sit between storage class and type
> >>
> >>
> >> Well, 'noinline' is different from 'inline'.
> >>
> >>  'noinline' is defined as:
> >>
> >>  #define  noinline                       __attribute__((noinline))
> >>
> >>  in include/linux/compiler-gcc.h. But 'inline' is a _keyword_ defined
> >>  by C standard. If checkpatch.pl complains about 'noinline', you should
> >>  fix checkpatch.pl. :)
> >Thanks, for explanation. But isn't it nice to place it between storage
> >class and type ?
> 
> I don't think so, I don't know why checkpatch.pl prefers that style.
> I think probably only because that is more readable?
> 

I think it's good for consistency reasons.  Yes, we _could_ have a
random sprinkling of different keyword orderings, but what benefit is
there in that?  In the great majority of places the kernel uses `static
inline void' and `static noinline void' ordering, and that's a good
thing, no?

So I merged the patch and I'd support retaining the checkpatch warning.

My one concern is that the patch is too small.  Probably there are
other codesites which get the keywords in a non-standard order, and I'd
rather fix them up in a single big pass rather than in a long series of
little patches.

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/