Bug 21080

Summary: poppler/CairoFontEngine.cc fails to build due to munmap()
Product: poppler Reporter: Darren Kenny <darren.kenny>
Component: generalAssignee: poppler-bugs <poppler-bugs>
Status: RESOLVED FIXED QA Contact:
Severity: normal    
Priority: medium    
Version: unspecified   
Hardware: All   
OS: Solaris   
Whiteboard:
i915 platform: i915 features:
Attachments: Patch to fix compilation issue.
Try again with #ifdefs...

Description Darren Kenny 2009-04-07 01:44:43 UTC
Created attachment 24633 [details] [review]
Patch to fix compilation issue.

The file poppler/CairoFontEngine.cc from 0.10.5 (and trunk) doesn't build on Solaris due to the way that munmap is called with an unsigned char* as opposed to a char* as the first parameter. 

The fix is to add a suitable cast to the function call, as in the attached patch.
Comment 1 Albert Astals Cid 2009-04-07 15:05:45 UTC
My munmap man page says
 int munmap(void *addr, size_t length);

And sun docs agree http://docs.sun.com:80/app/docs/doc/816-5167/munmap-2?a=view

Comment 2 Darren Kenny 2009-04-08 03:20:53 UTC
Hmm, it would appear that the definition changes depending on some #defines... 

From /usr/include/sys/mman.h:

#if (_POSIX_C_SOURCE > 2) || defined(_XPG4_2)
...
extern int munmap(void *, size_t);
...
#else	/* (_POSIX_C_SOURCE > 2) || defined(_XPG4_2) */
...
extern int munmap(caddr_t, size_t);
...

It looks like for C++ these macros are not defined since they conflict with the ANSI C++ standards - from standards manpage:

  ANSI/ISO C++
     ANSI/ISO C++ does not define any feature test macros. If the
     standard C++ announcement macro __cplusplus is predefined to
     value  199711  or	greater,  the  compiler	 operates  in	a
     standard-conforming  mode,	 indicating C++	standards confor-
     mance. The	value 199711  indicates	 conformance  to  ISO/IEC
     14882:1998,  as required by that standard.	 (As noted above,
     conformance to the	standard  is  incomplete.)   A	standard-
     conforming	mode is	not available with compilers prior to Sun
     WorkShop C++ 5.0.


     C++ bindings are not defined for POSIX  or	 X/Open	 CAE,  so
     specifying	  feature  test	 macros	 such  as  _POSIX_SOURCE,
     _POSIX_C_SOURCE, and _XOPEN_SOURCE	can result in compilation
     errors  due  to conflicting requirements of standard C++ and
     those specifications.

It would appear to not be a bug after all, I just need to figure out the correct definitions for Solaris.

Apologies, closing as not a bug.
Comment 3 Darren Kenny 2009-04-08 04:03:11 UTC
Created attachment 24664 [details] [review]
Try again with #ifdefs...

I've changed my mind, this is still an issue - since defining any of the "standards" macros causes more serious breakages else where. 

As a compromise, I've put a specific #ifdef test around the munmap call to meet the Sun Compiler's settings, so it should have no effect else where.
Comment 4 Darren Kenny 2009-04-08 04:04:26 UTC
Reopening since no mix of #defs allows all of poppler to compile cleanly. It makes more sense to fix specifically for the Sun Compiler.
Comment 5 Albert Astals Cid 2009-04-08 13:06:12 UTC
Carlos, what you think of this?
Comment 6 Carlos Garcia Campos 2009-04-10 04:24:53 UTC
if data->bytes is an unsigned char * and munmap expects a char * why not just adding the cast without any defines?
Comment 7 Albert Astals Cid 2009-04-10 04:40:39 UTC
Because of comment #1
Comment 8 Carlos Garcia Campos 2009-04-10 06:29:45 UTC
hmm, then we should use (void *) instead of (char *), no?
Comment 9 Albert Astals Cid 2009-04-10 07:20:39 UTC
But it seems some Solaris version and some Solaris compiler doesn't like void*
Comment 10 Carlos Garcia Campos 2009-04-10 08:03:45 UTC
strange. Well, if the patch fixes the problem and doesn't break any other compilers/platforms, it's ok to me. 
Comment 11 Albert Astals Cid 2009-04-10 09:08:28 UTC
Fixed in git

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.