Bug 105766 - Add missing time.h to some headers
Summary: Add missing time.h to some headers
Alias: None
Product: poppler
Classification: Unclassified
Component: general (show other bugs)
Version: unspecified
Hardware: Other All
: medium normal
Assignee: poppler-bugs
QA Contact:
Depends on:
Reported: 2018-03-27 12:17 UTC by Mojca Miklavec
Modified: 2018-04-02 11:50 UTC (History)
0 users

See Also:
i915 platform:
i915 features:

Add missing time.h plus minor cleanup (1.17 KB, patch)
2018-03-27 12:17 UTC, Mojca Miklavec
Details | Splinter Review

Description Mojca Miklavec 2018-03-27 12:17:32 UTC
Created attachment 138376 [details] [review]
Add missing time.h plus minor cleanup

I'm attaching a patch to fix a broken build on some platforms (notable FreeBSD and NetBSD). Since the time headers are needed in gfile.h not just in gfile.cc, I moved inclusion of time.h there and made a minor cleanup of code.

Note that #include <ctime.h> does not exist (not even on Mac), it's either <ctime> or <time.h>. I tested it and that fails. That said, #elif defined(MACOS) is not defined either, so it doesn't really do any harm, but it's of no use.

I had to patch poppler/Form.h in the released version because it also lacked time headers. When preparing the patch I noticed it was already fixed upstream, but if all other headers are included like <set> and <vector>, meaning that it's C++ anyway, so it also made more sense to use <ctime> rather than <time.h>. That one is optional though, it's just cosmetics.
Comment 1 Albert Astals Cid 2018-04-02 10:54:36 UTC
Thanks for the patch :)

I've commited only the compile fix.

The MACOS thing is interesting and someone should really have a look at all those ifdefs and see if we can just simply remove them or what.

The other one as you said it's cosmetics and i didn't change it.
Comment 2 Mojca Miklavec 2018-04-02 11:50:52 UTC
Thank you.

The #ifdef MACOS is in fact used all over the place and I have absolutely no clue what it does. It could be that some ancient version of xpdf would add that definition somehow to help compilation on super old systems. See https://sourceforge.net/p/predef/wiki/OperatingSystems/ for existing definitions. Even if MACOS was defined on version Mac OS 8 or 9 (which probably wasn't the case), this would not really help anyone.

Poppler can only be compiled on a relatively new macOS version (10.9 or newer out of the box, potentially on older ones with a lot of additional effort, but I bet nobody would dare to compile it on 10.3). I see there are some parts of the code related to different newline character (CR instead of LF). See https://superuser.com/a/439443

I bet that any code doing #ifdef MACOS can be removed without doing any harm whatsoever. But sure, a separate commit makes sense.

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.