[poppler] [Fwd: Some poppler patches for Win32 portability]

[Krzysztof Kowalczyk]

The functionality looks good. I have few implementation comments.

1. It should be explicitly GetModuleFileNameA() - otherwise in Unicode
build GetModeleFileName() will be resolved to GetModuleFileNameW() and
bad things will happen.

2. It assumes that poppler is built as a DLL. There is at least one
poppler user (my viewer
http://blog.kowalczyk.info/software/sumatrapdf/) that links poppler
code statically. With a small change to semantics, DllMain could be
removed and NULL passed to GetModuleFileNameA(). The difference is
that it would return path of the exe that loaded dll, instead of dll
itself (which, in most cases, will be the same).

3. It would be better to use GooString instead of a static buffer for
dirname, to avoid possible buffer overruns.

4. It's my little pet peeve about poppler code in general that is
repeated here: dir doesn't have to be new'ed and delete'ed, it can be
allocated on the stack (at least it should be - I vaguely remember
some class in poppler wasn't capable of being put on a stack, but even
if that is the case with GDir, it's a bug that can be fixed).

-- kjk

On Nov 26, 2007 7:36 AM, Kristian Høgsberg <krh at redhat.com> wrote:
> Hi Tor,
>
> Thanks for the patches, on a quick read-through they look fine.  I'm
> forwarding it to the poppler list so the people working on win32
> integration there can weigh in, but your changes look good to commit to
> me.
>
> cheers,
> Kristian
>
>
> ---------- Forwarded message ----------
> From: "Tor Lillqvist" <tml at iki.fi>
> To: krh at redhat.com
> Date: Thu, 22 Nov 2007 22:11:35 +0200
> Subject: Some poppler patches for Win32 portability
> Hi,
>
> I just built poppler on Windows with mingw. Seems to work fine, at
> least GIMP's PDF import plugin that uses poppler works as
> expected. The following patches were necessary:
>
> The first patch makes libpoppler not use hardcoded paths to the
> DATADIR on Windows. Instead the DLL looks up its installation location
> at run-time and constructs the path to the share/poppler folder from
> that.
>
> patch -p0 <<'EOF'
> --- poppler/GlobalParams.cc~    2007-11-05 01:11:04.000000000 +0200
> +++ poppler/GlobalParams.cc     2007-11-22 21:34:33.959200600 +0200
> @@ -22,6 +22,7 @@
>  #endif
>  #ifdef WIN32
>  #  include <shlobj.h>
> +#  include <mbstring.h>
>  #endif
>  #include <fontconfig/fontconfig.h>
>  #include "goo/gmem.h"
> @@ -76,6 +77,59 @@
>  #  endif
>  #endif
>
> +#ifdef WIN32
> +static HMODULE hmodule;
> +
> +extern "C" {
> +BOOL WINAPI
> +DllMain (HINSTANCE hinstDLL,
> +        DWORD     fdwReason,
> +        LPVOID    lpvReserved)
> +{
> +  switch (fdwReason)
> +    {
> +    case DLL_PROCESS_ATTACH:
> +      hmodule = hinstDLL;
> +      break;
> +    }
> +
> +  return TRUE;
> +}
> +}
> +
> +static char *
> +get_poppler_datadir (void)
> +{
> +  static char retval[1000];
> +  static int beenhere = 0;
> +
> +  unsigned char *p;
> +
> +  if (beenhere)
> +    return retval;
> +
> +  if (!GetModuleFileName (hmodule, (CHAR *) retval, sizeof(retval) - 10))
> +    return POPPLER_DATADIR;
> +
> +  p = _mbsrchr ((const unsigned char *) retval, '\\');
> +  *p = '\0';
> +  p = _mbsrchr ((const unsigned char *) retval, '\\');
> +  if (p) {
> +    if (stricmp ((const char *) (p+1), "bin") == 0)
> +      *p = '\0';
> +  }
> +  strcat (retval, "\\share\\poppler");
> +
> +  beenhere = 1;
> +
> +  return retval;
> +}
> +
> +#undef POPPLER_DATADIR
> +#define POPPLER_DATADIR get_poppler_datadir ()
> +
> +#endif
> +
>  //------------------------------------------------------------------------
>
>  #define cidToUnicodeCacheSize     4
> @@ -653,8 +707,11 @@
>  void GlobalParams::scanEncodingDirs() {
>    GDir *dir;
>    GDirEntry *entry;
> +  char dirname[1000];
>
> -  dir = new GDir(POPPLER_DATADIR "/nameToUnicode", gTrue);
> +  strcpy(dirname, POPPLER_DATADIR);
> +  strcat(dirname, "/nameToUnicode");
> +  dir = new GDir(dirname, gTrue);
>    while (entry = dir->getNextEntry(), entry != NULL) {
>      if (!entry->isDir()) {
>        parseNameToUnicode(entry->getFullPath());
> @@ -663,21 +720,27 @@
>    }
>    delete dir;
>
> -  dir = new GDir(POPPLER_DATADIR "/cidToUnicode", gFalse);
> +  strcpy(dirname, POPPLER_DATADIR);
> +  strcat(dirname, "/cidToUnicode");
> +  dir = new GDir(dirname, gFalse);
>    while (entry = dir->getNextEntry(), entry != NULL) {
>      addCIDToUnicode(entry->getName(), entry->getFullPath());
>      delete entry;
>    }
>    delete dir;
>
> -  dir = new GDir(POPPLER_DATADIR "/unicodeMap", gFalse);
> +  strcpy(dirname, POPPLER_DATADIR);
> +  strcat(dirname, "/unicodeMap");
> +  dir = new GDir(dirname, gFalse);
>    while (entry = dir->getNextEntry(), entry != NULL) {
>      addUnicodeMap(entry->getName(), entry->getFullPath());
>      delete entry;
>    }
>    delete dir;
>
> -  dir = new GDir(POPPLER_DATADIR "/cMap", gFalse);
> +  strcpy(dirname, POPPLER_DATADIR);
> +  strcat(dirname, "/cMap");
> +  dir = new GDir(dirname, gFalse);
>    while (entry = dir->getNextEntry(), entry != NULL) {
>      addCMapDir(entry->getName(), entry->getFullPath());
>      toUnicodeDirs->append(entry->getFullPath()->copy());
> EOF
>
> There is no localtime_r() in the Microsoft C library, but their
> localtime() is actually thread-safe. (It uses a thread-local buffer.)
>
> patch -p0 <<'EOF'
> --- glib/demo/info.c~   2007-11-05 01:11:00.000000000 +0200
> +++ glib/demo/info.c    2007-11-22 20:38:38.568575600 +0200
> @@ -21,6 +21,10 @@
>  #include "info.h"
>  #include "utils.h"
>
> +#ifdef G_OS_WIN32
> +#define localtime_r(tp,tmp) (localtime(tp) ? (*(tmp) = *localtime
> (tp), (tmp)) : NULL)
> +#endif
> +
>  static gchar *
>  poppler_format_date (GTime utime)
>  {
> EOF
>
> Use -no-undefined also when building libpoppler-glib.
>
> patch -p0 <<'EOF'
> --- glib/Makefile.in~   2007-11-10 14:04:33.000000000 +0200
> +++ glib/Makefile.in    2007-11-22 20:43:40.006075600 +0200
> @@ -330,7 +330,7 @@
>         $(FONTCONFIG_LIBS)                              \
>         $(cairo_libs)
>
> -libpoppler_glib_la_LDFLAGS = -version-info 2:0:0
> +libpoppler_glib_la_LDFLAGS = -version-info 2:0:0 @create_shared_lib@
>  test_poppler_glib_SOURCES = \
>         test-poppler-glib.cc
>
> EOF
>
> In addition I had to add the path to where I have jpeglib.h manually
> to the configure script. For the stuff in glib/demo to build, I had to
> use -Wl,--enable-runtime-pseudo-reloc in my LDFLAGS. Handling these
> things could be added to the autoconfusion but I was too lazy for that
> for now...
>
> (In case gmail mishandles the patches inline, just ask and I can mail
> them as an attachment instead.)
>
> Cheers,
> --tml
>
> _______________________________________________
> poppler mailing list
> poppler at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/poppler
>
>