Questionable inline usage in gettext.h

[Nick Bowler]

Hi folks,

I'm using Gnulib's gettext module for i18n, and I've noticed some
questionable use of "inline" in lib/gettext.h.  For reference, I'm
quoting one function from that header file (comments follow):

> #define pgettext_expr(Msgctxt, Msgid) \
>   dcpgettext_expr (NULL, Msgctxt, Msgid, LC_MESSAGES)
> #define dpgettext_expr(Domainname, Msgctxt, Msgid) \
>   dcpgettext_expr (Domainname, Msgctxt, Msgid, LC_MESSAGES)
> 
> #ifdef __GNUC__
> __inline
> #else
> #ifdef __cplusplus
> inline
> #endif
> #endif
> static const char *
> dcpgettext_expr (const char *domain,
>                  const char *msgctxt, const char *msgid,
>                  int category)
> {
>   size_t msgctxt_len = strlen (msgctxt) + 1;
>   size_t msgid_len = strlen (msgid) + 1;
>   const char *translation;
> #if _LIBGETTEXT_HAVE_VARIABLE_SIZE_ARRAYS
>   char msg_ctxt_id[msgctxt_len + msgid_len];
> #else
>   char buf[1024];
>   char *msg_ctxt_id =
>     (msgctxt_len + msgid_len <= sizeof (buf)
>      ? buf
>      : (char *) malloc (msgctxt_len + msgid_len));
>   if (msg_ctxt_id != NULL)
> #endif
>     {
>       memcpy (msg_ctxt_id, msgctxt, msgctxt_len - 1);
>       msg_ctxt_id[msgctxt_len - 1] = '\004';
>       memcpy (msg_ctxt_id + msgctxt_len, msgid, msgid_len);
>       translation = dcgettext (domain, msg_ctxt_id, category);
> #if !_LIBGETTEXT_HAVE_VARIABLE_SIZE_ARRAYS
>       if (msg_ctxt_id != buf)
>         free (msg_ctxt_id);
> #endif
>       if (translation != msg_ctxt_id)
>         return translation;
>     }
>   return msgid;
> }

First and foremost, we notice that this function emits the "inline"
specifier (or equivalent) for C++ and when using the GNU C compiler.
Notably missing is a case for C99 (#if __STDC_VERSION__ >= 199901).
Moreover, one wonders why this doesn't simply rely on autoconf's
AC_C_INLINE machinery.

Secondly, this function is *big*.  With -Os, GCC produces more than 150
bytes of code for this function on my system.  This typically dominates
the function call overhead, so assuming it is actually inlined (more on
that below), we're looking at approximately 150 bytes of bloat for every
call to one of the pgettext_expr family of functions.

Thirdly, GCC will normally refuse to inline any function (such as the
above quoted one) that uses variable-length arrays anyway (compile with
-Winline to see this), so the use of the inline specifier seems at best
superfluous:

  ./lib/gettext.h:208:1: warning: function ‘dcpgettext_expr’ can never be 
inlined because it uses alloca (override using the always_inline attribute)

Finally, regardless of whether or not the function actually gets
inlined it's likely that this code will be duplicated at least in
every translation unit which calls it since it has internal linkage.
One wonders why these big functions aren't in their own .c file.

Cheers,
-- 
Nick Bowler, Elliptic Technologies (http://www.elliptictech.com/)