Closed
Bug 651444
Opened 13 years ago
Closed 13 years ago
qcmstypes.h redefines intX_t types already defined by sys/types.h
Categories
(Core :: Graphics: Color Management, defect)
Tracking
()
RESOLVED
FIXED
mozilla10
People
(Reporter: gaston, Assigned: gaston)
References
Details
Attachments
(2 files, 2 obsolete files)
831 bytes,
patch
|
jrmuizel
:
review+
emorley
:
checkin+
|
Details | Diff | Splinter Review |
893 bytes,
patch
|
jrmuizel
:
review+
emorley
:
checkin+
|
Details | Diff | Splinter Review |
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•13 years ago
|
Blocks: openbsdmeta
Assignee | ||
Comment 1•13 years ago
|
||
Maybe not the best fix, but at least there's a patch for review..
Attachment #560862 -
Flags: review?(jmuizelaar)
Assignee | ||
Comment 2•13 years ago
|
||
Or, a better fix might be #elif (!defined(ANDROID) && !defined(__OpenBSD__)), but given the spaghetti #ifdef maze this header is already....
Comment 3•13 years ago
|
||
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•13 years ago
|
||
Your call, here's a new one with that.
Assignee: nobody → landry
Attachment #560862 -
Attachment is obsolete: true
Attachment #561192 -
Flags: review?(jmuizelaar)
Updated•13 years ago
|
Attachment #561192 -
Flags: review?(jmuizelaar) → review+
Assignee | ||
Updated•13 years ago
|
Status: NEW → ASSIGNED
Keywords: checkin-needed
Comment 5•13 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/2718a99132a2
Flags: in-testsuite-
Keywords: checkin-needed
Target Milestone: --- → mozilla9
Version: unspecified → Trunk
Comment 6•13 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/2718a99132a2
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 7•13 years ago
|
||
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•13 years ago
|
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Updated•13 years ago
|
Attachment #561809 -
Flags: review?(jmuizelaar) → review+
Assignee | ||
Updated•13 years ago
|
Keywords: checkin-needed
Comment 8•13 years ago
|
||
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? :-)
Assignee | ||
Comment 10•13 years ago
|
||
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•13 years ago
|
||
And qcms part of the tree built fine with that diff..
Updated•13 years ago
|
Attachment #562795 -
Flags: review?(jmuizelaar) → review+
Assignee | ||
Updated•13 years ago
|
Keywords: checkin-needed
Comment 12•13 years ago
|
||
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+
Comment 13•13 years ago
|
||
(For whomever merges, both patches have now landed, ok to close on merge)
Keywords: checkin-needed
Target Milestone: mozilla9 → mozilla10
Updated•13 years ago
|
Attachment #561192 -
Flags: checkin+
Comment 14•13 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/c81e6b2dc155
Status: REOPENED → RESOLVED
Closed: 13 years ago → 13 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•