The default bug view has changed. See this FAQ.

qcmstypes.h redefines intX_t types already defined by sys/types.h

RESOLVED FIXED in mozilla10

Status

()

Core
GFX: Color Management
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: gaston, Assigned: gaston)

Tracking

Trunk
mozilla10
All
OpenBSD
Points:
---
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 2 obsolete attachments)

(Assignee)

Description

6 years ago
Currently qcms fails to build on OpenBSD :

gcc -o transform-sse2.o -c -I../../dist/system_wrappers -include /home/landry/src/mozilla-central/config/gcc_hidden.h -DMOZILLA_INTERNAL_AP
I -D_IMPL_NS_COM -DEXPORT_XPT_API -DEXPORT_XPTC_API -D_IMPL_NS_GFX -D_IMPL_NS_WIDGET -DIMPL_XREAPI -DIMPL_NS_NET -DIMPL_THEBES  -DSTATIC_EX
PORTABLE_JS_API -DOSTYPE=\"OpenBSD4\" -DOSARCH=OpenBSD  -I/home/landry/src/mozilla-central/gfx/qcms -I. -I../../dist/include -I../../dist/i
nclude/nsprpub  -I/usr/ports/pobj/m-c/dist/include/nspr -I/usr/ports/pobj/m-c/dist/include/nss       -fPIC -I/usr/X11R6/include -Wall -W -W
no-unused -Wpointer-arith -Wcast-align -W -pedantic -Wno-long-long -fno-strict-aliasing -pthread -pipe  -DNDEBUG -DTRIMMED -g -O -DMOZ_QCMS
 -Wno-missing-field-initializers  -I/usr/X11R6/include -include ../../mozilla-config.h -DMOZILLA_CLIENT -MD -MF .deps/transform-sse2.pp -ms
se2 /home/landry/src/mozilla-central/gfx/qcms/transform-sse2.c
In file included from /home/landry/src/mozilla-central/gfx/qcms/qcmsint.h:1,
                 from /home/landry/src/mozilla-central/gfx/qcms/iccread.c:29:
/home/landry/src/mozilla-central/gfx/qcms/qcms.h:75: warning: ISO C restricts enumerator values to range of 'int'
In file included from /home/landry/src/mozilla-central/gfx/qcms/qcmsint.h:2,
                 from /home/landry/src/mozilla-central/gfx/qcms/iccread.c:29:
/home/landry/src/mozilla-central/gfx/qcms/qcmstypes.h:22: error: conflicting types for 'int64_t'
/usr/include/sys/types.h:102: error: previous declaration of 'int64_t' was here
/home/landry/src/mozilla-central/gfx/qcms/qcmstypes.h:23: error: conflicting types for 'uint64_t'
/usr/include/sys/types.h:107: error: previous declaration of 'uint64_t' was here

sys/types.h happens to be included before prtypes.h, which messes things around.

My current workaround would be to _not_ redefine basic types in qcmstypes.h (ie add OpenBSD to the #if !ANDROID block), but i'm not sure it's the best fix. How come sys/types.h is included first, and how come so many headers like to redefine systemwide types ?
(Assignee)

Updated

6 years ago
Blocks: 650665
(Assignee)

Comment 1

6 years ago
Created attachment 560862 [details] [diff] [review]
Don't redefine int*_t types on OpenBSD, they come from sys/types.h

Maybe not the best fix, but at least there's a patch for review..
Attachment #560862 - Flags: review?(jmuizelaar)
(Assignee)

Comment 2

6 years ago
Or, a better fix might be #elif (!defined(ANDROID) && !defined(__OpenBSD__)), but given the spaghetti #ifdef maze this header is already....
Comment on attachment 560862 [details] [diff] [review]
Don't redefine int*_t types on OpenBSD, they come from sys/types.h

I would prefer #elif !defined(ANDROID) && !defined(__OpenBSD__)
Attachment #560862 - Flags: review?(jmuizelaar) → review-
(Assignee)

Comment 4

6 years ago
Created attachment 561192 [details] [diff] [review]
Don't redefine int*_t types on OpenBSD, they come from sys/types.h

Your call, here's a new one with that.
Assignee: nobody → landry
Attachment #560862 - Attachment is obsolete: true
Attachment #561192 - Flags: review?(jmuizelaar)
Attachment #561192 - Flags: review?(jmuizelaar) → review+
(Assignee)

Updated

6 years ago
Status: NEW → ASSIGNED
Keywords: checkin-needed
https://hg.mozilla.org/integration/mozilla-inbound/rev/2718a99132a2
Flags: in-testsuite-
Keywords: checkin-needed
Target Milestone: --- → mozilla9
Version: unspecified → Trunk
https://hg.mozilla.org/mozilla-central/rev/2718a99132a2
Status: ASSIGNED → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
(Assignee)

Comment 7

6 years ago
Created attachment 561809 [details] [diff] [review]
include inttypes.h for uintptr_t

Doh, i knew that last minute change was going to bite me. as openbsd is in the same ifdef as android, it doesn't define uintptr_t anymore, while my initial patch didn't reach that far. So now the build fails with missing uintptr_t type.

Here's a patch that correctly includes inttypes.h, instead of doing one more typedef ...
Attachment #561809 - Flags: review?(jmuizelaar)
(Assignee)

Updated

6 years ago
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Attachment #561809 - Flags: review?(jmuizelaar) → review+
(Assignee)

Updated

6 years ago
Keywords: checkin-needed
Comment on attachment 561809 [details] [diff] [review]
include inttypes.h for uintptr_t

Review of attachment 561809 [details] [diff] [review]:
-----------------------------------------------------------------

::: gfx/qcms/qcmstypes.h
@@ +17,1 @@
>  #elif !defined(ANDROID) && !defined(__OpenBSD__)

The |&& !defined(__OpenBSD__)| on line 17 can be taken out now surely? :-)
Removing checkin-needed for comment 8.
Keywords: checkin-needed
(Assignee)

Comment 10

6 years ago
Created attachment 562795 [details] [diff] [review]
include inttypes.h for uintptr_t

Doh, indeed. Here's the final diff i'm trying, results in a few.
Attachment #561809 - Attachment is obsolete: true
Attachment #562795 - Flags: review?(jmuizelaar)
(Assignee)

Comment 11

6 years ago
And qcms part of the tree built fine with that diff..
Attachment #562795 - Flags: review?(jmuizelaar) → review+
(Assignee)

Updated

6 years ago
Keywords: checkin-needed
Comment on attachment 562795 [details] [diff] [review]
include inttypes.h for uintptr_t

https://hg.mozilla.org/integration/mozilla-inbound/rev/c81e6b2dc155
Attachment #562795 - Flags: checkin+
(For whomever merges, both patches have now landed, ok to close on merge)
Keywords: checkin-needed
Target Milestone: mozilla9 → mozilla10
Attachment #561192 - Flags: checkin+
https://hg.mozilla.org/mozilla-central/rev/c81e6b2dc155
Status: REOPENED → RESOLVED
Last Resolved: 6 years ago6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.