This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: PATCH: Use __builtin_bswap32/64 for GCC >= 4.2


> __extern_inline  doesn't work with all compilers.  

Please explain.  It's used in lots of places already.  If there is a
problem with some compilers, we should fix the macro definition in
misc/sys/cdefs.h.  In general, extern inline is the right choice.
If the compiler decides not to inline a function, then it should be
gotten from the DSO.  For functions so tiny that's it doesn't seem
to make any sense for the compiler to fail to inline (which seems
likely for the bswap cases), we have __always_inline.  (Note that
__extern_always_inline also uses __artificial__ and thus should
really only be used for the _FORTIFY_SOURCE cases where hiding the
function is what we want.  IMHO it would make sense to change the
name of that macro to indicate its true purpose.)

> I checked all user-visible glibc header files with inline functions.
> They are declared with static __inline, __extern_always_inline or
> __extern_inline.  None of them use __attribute__ ((__unused__)).  If
> static __inline should be changed, I believe it should be done with a
> separate patch.

Ok.  I guess the system headers exception is taking caring of
-Wunused issues already.  So there is nothing new here.  While I
think it would be better to use __attribute__ ((__unused__))
explicitly on all static inlines in public headers--and really, not
to use static inline at all but to use __extern_inline instead, with
__always_inline when appropriate--that is orthogonal to your changes
and can wait for later.


Thanks,
Roland




Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]