Bug 104045

Created attachment 135889 [details] [review]
Fix crash in glib demo in mingw

Missing NULL check

Created attachment 135890 [details] [review]
Enable all libraries on windows

Some libraries are enabled, some disabled on windows which makes mingw testing harder. I see no reason why they can't all be enabled. It builds fine with cygwin/mingw.

Created attachment 135891 [details] [review]
Remove unused cmake modules

Created attachment 135892 [details] [review]
-DUSE_OPENJPEG2 is not used

Created attachment 135893 [details] [review]
Move UTF functions to goo

Move the UTF conversion functions from poppler/UTF.cc to goo/UTF.cc so they can be used in goo/gfile.cc.

In future we could potentially include UTF helper functions in GooString to simply all the code that does a two step convert the UTF then store in GooString.

The remaining code in poppler/UTF.cc has been renamed to poppler/Unicode.cc to avoid having two files with the same name.

Created attachment 135894 [details] [review]
Fix compile warning on mingw

Now that I've enabled GTK on windows, fix a compile warning in the glib demo.

Created attachment 135895 [details] [review]
win32 support unicode filenames in GDir/GDirEntry

Created attachment 135896 [details] [review]
Make GDir implementation private

Created attachment 135897 [details] [review]
Replace UTF gmalloc/gfree with new/delete

Created attachment 135898 [details] [review]
Replace open coded UTF8 conversion with a call to UTF.cc

Created attachment 135899 [details] [review]
Remove unused gfile functions

As far as I can see, these functions originated in xpdf and have never been used in poppler.

Created attachment 135900 [details] [review]
Make char* version of GooFile::open() support unicode

Created attachment 135901 [details] [review]
Remove unused getFileNameU() function

What's your opinion on supporting or removing some of the old platform macros in gfile.cc?

My understanding is MACOS is all POSIX now so the MACOS macros should not be required. I don't know if anyone uses or if it is even possible to compile poppler on ACORN or VMS, or OS/2.

(In reply to Adrian Johnson from comment #1)
> Created attachment 135889 [details] [review] [review]
> Fix crash in glib demo in mingw
> 
> Missing NULL check

Looks good, please commit :)

(In reply to Adrian Johnson from comment #2)
> Created attachment 135890 [details] [review] [review]
> Enable all libraries on windows
> 
> Some libraries are enabled, some disabled on windows which makes mingw
> testing harder. I see no reason why they can't all be enabled. It builds
> fine with cygwin/mingw.

I've no idea why they are disabled, i'm going to say "maybe MSVC", but since i've no clue, you can commit it and i'll blame you if someone else comes saying it broke for them :D

(In reply to Adrian Johnson from comment #3)
> Created attachment 135891 [details] [review] [review]
> Remove unused cmake modules

Looks good, though i already removed cmake/modules/FindLIBOPENJPEG.cmake at some point, so you'll have to rebase/merge/something

(In reply to Adrian Johnson from comment #4)
> Created attachment 135892 [details] [review] [review]
> -DUSE_OPENJPEG2 is not used

Please commit :)

(In reply to Adrian Johnson from comment #5)
> Created attachment 135893 [details] [review] [review]
> Move UTF functions to goo
> 
> Move the UTF conversion functions from poppler/UTF.cc to goo/UTF.cc so they
> can be used in goo/gfile.cc.
> 
> In future we could potentially include UTF helper functions in GooString to
> simply all the code that does a two step convert the UTF then store in
> GooString.
> 
> The remaining code in poppler/UTF.cc has been renamed to poppler/Unicode.cc
> to avoid having two files with the same name.

I didn't check the code, but if it's just a move, sure go ahead.

(In reply to Adrian Johnson from comment #6)
> Created attachment 135894 [details] [review] [review]
> Fix compile warning on mingw
> 
> Now that I've enabled GTK on windows, fix a compile warning in the glib demo.

The goo/glibc.h patch is "wrong", it's been fixed already in a different way.

(In reply to Adrian Johnson from comment #7)
> Created attachment 135895 [details] [review] [review]
> win32 support unicode filenames in GDir/GDirEntry

I've no idea nor really much itnerested about caring for windows, if you think it's fine and will fix it if it's not, just commit :)

(In reply to Adrian Johnson from comment #8)
> Created attachment 135896 [details] [review] [review]
> Make GDir implementation private

looks good :)

(In reply to Adrian Johnson from comment #9)
> Created attachment 135897 [details] [review] [review]
> Replace UTF gmalloc/gfree with new/delete

Any particular reason for this? Also the correct way to delete a new[] is delete[] not delete.

(In reply to Adrian Johnson from comment #10)
> Created attachment 135898 [details] [review] [review]
> Replace open coded UTF8 conversion with a call to UTF.cc

No idea of what the old code does, but less code looks good :)

(In reply to Adrian Johnson from comment #11)
> Created attachment 135899 [details] [review] [review]
> Remove unused gfile functions
> 
> As far as I can see, these functions originated in xpdf and have never been
> used in poppler.

kill them with fire!

(In reply to Adrian Johnson from comment #12)
> Created attachment 135900 [details] [review] [review]
> Make char* version of GooFile::open() support unicode

this only touches windows code, right? If so feel free to commit :)

(In reply to Adrian Johnson from comment #13)
> Created attachment 135901 [details] [review] [review]
> Remove unused getFileNameU() function

Looks good.

(In reply to Adrian Johnson from comment #14)
> What's your opinion on supporting or removing some of the old platform
> macros in gfile.cc?
> 
> My understanding is MACOS is all POSIX now so the MACOS macros should not be
> required. I don't know if anyone uses or if it is even possible to compile
> poppler on ACORN or VMS, or OS/2.

How painful is to leave them around, seem that acorn/vms and to a lesser extend os/2 are still "kind of alive", i've no idea if anyone has tried building them, but if it's not too much of a pain to keep the defines i'd let them just live.

-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/poppler/poppler/issues/554.