[poppler] strndup is not universally available

Kristian Høgsberg krh at bitplanet.net
Mon Jan 28 14:45:02 PST 2008


On Jan 27, 2008 7:01 PM, Jonathan Kew <jonathan_kew at sil.org> wrote:
> The patch in December to support the Adobe Glyph Naming convention
> introduced the use of strndup() into GfxFont.cc. However, strndup is
> a GNU library extension, not universally available and not defined in
> POSIX, AFAIK.
>
> Therefore, I suggest that it should be checked at configure time, and
> an alternative provided for systems that lack this function:
>
> --- a/configure.ac
> +++ b/configure.ac
> @@ -17,6 +17,7 @@ AC_PROG_CXX
>   AC_PROG_INSTALL
>   AC_CHECK_FUNC(gettimeofday, AC_DEFINE(HAVE_GETTIMEOFDAY, 1,
> [Defines if gettimeofday is available on your system]))
>   AC_CHECK_FUNC(localtime_r, AC_DEFINE(HAVE_LOCALTIME_R, 1, [Defines
> if localtime_r is available on your system]))
> +AC_CHECK_FUNC(strndup, AC_DEFINE(HAVE_STRNDUP, 1, [Defines if
> strndup is available on your system]))
>
>   dnl Enable these unconditionally.
>   AC_DEFINE([OPI_SUPPORT], [1], [Generate OPI comments in PS output.])
>
> --- a/poppler/GfxFont.cc
> +++ b/poppler/GfxFont.cc
> @@ -974,7 +974,13 @@ static int parseCharName(char *charName, Unicode
> *uBuf, int uLen,
>         return 0;       // .notdef or similar
>       } else if (var_part != NULL) {
>         // parse names of the form 7.oldstyle, P.swash, s.sc, etc.
> +#ifdef HAVE_STRNDUP
>         char *main_part = strndup(charName, var_part - charName);
> +#else
> +      char *main_part = (char*)gmalloc(var_part - charName + 1);
> +      main_part[var_part - charName] = '\0';
> +      (void)memcpy(main_part, charName, var_part - charName);
> +#endif
>         GBool namesRecurse = gTrue, variantsRecurse = gFalse;
>         int n = parseCharName(main_part, uBuf, uLen, namesRecurse,
> ligatures,

I see Albert already applied your patch, but this is really a pet
peeve of mine.  DO NOT litter the code with #ifdef's like this and DO
NOT leave open coded strndup implementations around the code base.  I
mean, do you not see how unmanageble the code base becomes if future
uses of strndup follow this pattern?  The right fix is to split the
code out into its own function and put it in goo/gmem.c along with the
other memory/string functions there.

The patch does reveal a bug in the existing code - the strndup()
allocated string is freed with gfree(), but the memory returned by
strndup() should be freed with free().

Kristian


More information about the poppler mailing list