Bug 57687 - pdftocairo shows wrong version on cygwin
Summary: pdftocairo shows wrong version on cygwin
Status: RESOLVED FIXED
Alias: None
Product: poppler
Classification: Unclassified
Component: utils (show other bugs)
Version: unspecified
Hardware: Other All
: medium normal
Assignee: poppler-bugs
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-11-29 13:36 UTC by Adrian Johnson
Modified: 2012-12-08 22:33 UTC (History)
0 users

See Also:
i915 platform:
i915 features:


Attachments
Move jpeglib.h into .cc file (8.06 KB, text/plain)
2012-11-29 13:36 UTC, Adrian Johnson
Details
reformat goo/*Writer files (19.61 KB, patch)
2012-12-01 23:08 UTC, Adrian Johnson
Details | Splinter Review
Move jpeglib.h into .cc file (8.03 KB, patch)
2012-12-01 23:11 UTC, Adrian Johnson
Details | Splinter Review

Description Adrian Johnson 2012-11-29 13:36:26 UTC
Created attachment 70790 [details]
Move jpeglib.h into .cc file

On cygwin pdftocairo displays "pdftocairo version 8.0.2".

The problem is /usr/include/jconfig.h (included by jpeglib.h) on cygwin defines PACKAGE_VERSION.

The attached patch fixes this by moving the #include <jpeglib.h> and libjpeg types from JpegWriter.h to JpegWriter.cc. This avoids polluting the name space of our internal header files with junk from other libraries.
Comment 1 Albert Astals Cid 2012-12-01 00:56:22 UTC
Can you have a look at the spacing? seems some parts are using spaces and some other tabs.

Also i'd be cool if you declare the copy constructor and the assignment operator as private without implementing so we don't end up using the default ones by mistake (since using them would create very bad things) (Look at GooString if you are unsure of what i mean)
Comment 2 Adrian Johnson 2012-12-01 23:08:46 UTC
Created attachment 70896 [details] [review]
reformat goo/*Writer files

I've reformatted the goo/*Writer files to be consistent with the poppler style of 2 space indent, tabs enabled, tab = 8.
Comment 3 Adrian Johnson 2012-12-01 23:11:09 UTC
Created attachment 70897 [details] [review]
Move jpeglib.h into .cc file

Updated patch to make copy constructor and assignment operator private.
Comment 4 Albert Astals Cid 2012-12-08 16:51:43 UTC
Works for me, two minor things, feel free to fix them or not when pushing

The if !priv in ::init feels a bit to much C-ish to me since we initialize priv in all the constructors

Usually when one uses the FooPrivate pattern the Private class/structs holds all the data members of the class while in this case it only holds the jpeg_ ones

As said feel free to fix or not and push
Comment 5 Adrian Johnson 2012-12-08 22:33:01 UTC
I've removed the !priv in ::init and moved all data members to JpegPrivate.

Pushed.


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.