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 <[EMAIL PROTECTED]> 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" <[EMAIL PROTECTED]> > To: [EMAIL PROTECTED] > 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 > [email protected] > http://lists.freedesktop.org/mailman/listinfo/poppler > > _______________________________________________ poppler mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/poppler
