Closed Bug 631513 Opened 13 years ago Closed 13 years ago

ots fails to compile on mingw

Categories

(Core :: Graphics, defect)

x86
Windows XP
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: jacek, Assigned: jacek)

Details

Attachments

(1 file, 1 obsolete file)

Attached patch fix v1.0 (obsolete) — Splinter Review
It's fixed in mainstream: http://code.google.com/p/chromium/issues/detail?id=59114
As there probably won't be mainstream pull for a while and it's currently the only patch required to build functional browser from m-c, I think it's worth to commit it now.
Attachment #509742 - Flags: review?(jfkthame)
Comment on attachment 509742 [details] [diff] [review]
fix v1.0

IIRC, mingw provides <stdint.h>, doesn't it? If so, it seems better to use the standard header rather than the typedefs here, and _MSC_VER would be the correct symbol to check for that decision.

The _WIN32 condition would then only be needed for the <winsock.h>, I think. And maybe the #undef's of min and max, if those are a problem under mingw.

(I don't have a mingw build setup to check this, but judging by http://mxr.mozilla.org/mozilla-central/source/gfx/harfbuzz/src/hb-common.h#41 I'm assuming <stdint.h> is ok to use on mingw.)
Attached patch fix v1.1Splinter Review
Thanks for the review. I previously wanted the least invasion fix, but yeah, using stdint.h seems better. I've checked and undefining min/max macros is not needed for mingw (BTW, I actually use fork of mingw, mingw-w64), but I think we should rather keep #undefs for mingw. The problem may appear once they will change headers to be more compatible with MSVC.
Attachment #509742 - Attachment is obsolete: true
Attachment #509752 - Flags: review?(jfkthame)
Attachment #509742 - Flags: review?(jfkthame)
Comment on attachment 509752 [details] [diff] [review]
fix v1.1

Thanks, looks fine to me. (Maybe worth proposing the same patch upstream?)
Attachment #509752 - Flags: review?(jfkthame) → review+
Sent to upstream: http://codereview.chromium.org/6312175/
Attachment #509752 - Flags: approval2.0?
Attachment #509752 - Flags: approval2.0? → approval2.0+
(Approved so long as this passes try!)
Succeeded on try:
http://tbpl.mozilla.org/?tree=MozillaTry&rev=f374a65445b3

Pushed to m-c:
http://hg.mozilla.org/mozilla-central/rev/28bf1def9206
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.