xfs
[Top] [All Lists]

Re: [PATCH 1/2] Make stuff static

To: David Chinner <dgc@xxxxxxx>
Subject: Re: [PATCH 1/2] Make stuff static
From: Russell Cattelan <cattelan@xxxxxxxxxxx>
Date: Tue, 17 Oct 2006 17:45:31 -0500
Cc: Tim Shimmin <tes@xxxxxxx>, Eric Sandeen <sandeen@xxxxxxxxxxx>, xfs@xxxxxxxxxxx
In-reply-to: <20061017215706.GI8394166@melbourne.sgi.com>
References: <20060929032856.8DA9C18001A5E@sandeen.net> <23F15D6AE8566A54B81188AC@timothy-shimmins-power-mac-g5.local> <45338DDE.8020903@sandeen.net> <4533FAEA.2080500@sandeen.net> <20061016232250.GM11034@melbourne.sgi.com> <1161042943.5723.117.camel@xenon.msp.redhat.com> <20061017005038.GN11034@melbourne.sgi.com> <AED98B89E193744D39BAC541@pmmelb207.melbourne.sgi.com> <20061017215706.GI8394166@melbourne.sgi.com>
Sender: xfs-bounce@xxxxxxxxxxx
On Wed, 2006-10-18 at 07:57 +1000, David Chinner wrote:
> On Tue, Oct 17, 2006 at 05:13:01PM +1000, Tim Shimmin wrote:
> > --On 17 October 2006 10:50:38 AM +1000 David Chinner <dgc@xxxxxxx> wrote:
> > >Fix them - inline functions in header files should always be "static
> > >inline".  Inline functions in .c files should always be static as
> > >well - if they need to be accessed from different source files then
> > >they need to be in header files.  Hence "STATIC inline" is broken
> > >code and should be fixed anyway. Luckily, there are very few of
> > >these to fix and they are all in .c files:
> > >
> > >chook 137% grep -rIw "STATIC inline" fs/xfs | wc -l
> > >21
> > 
> > So you are saying that "static inline"s should always be.
> 
> Yup.
> 
> > So for CONFIG_XFS_DEBUG where we define STATIC and so make static
> > disappear for uses of STATIC, we will no longer touch these
> > "static inline" functions.
> 
> Yup. The function will still get inlined, so changing it's scope on
> debug builds doesn't provide any benefit IMO. FWIW, for debug
> builds we probably want noinline....
> 
> > I thought that for debug, we could stop them from being inline
> > for easier debugging. We could have a STATIC_INLINE :-)
> 
> We could, but I don't think it gains us anything.
I agree with Tim on this.
when I see STATIC in the code it's generally assumed to 
be a way to toggle of static on/off. Adding static inline 
to the #define STATIC starts to overload the the macro 
and creates an obfuscation that isn't immediately obvious.
STATIC_INLINE should be fairly obvious.


> 
> Cheers,
> 
> Dave.
-- 
Russell Cattelan <cattelan@xxxxxxxxxxx>

Attachment: signature.asc
Description: This is a digitally signed message part

<Prev in Thread] Current Thread [Next in Thread>