Closed Bug 896704 Opened 12 years ago Closed 12 years ago

Remove the MSVC exemption for FAIL_ON_WARNINGS in media/mtransport

Categories

(Core :: WebRTC, defect)

x86_64
Windows 8
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla28

People

(Reporter: emk, Assigned: emk)

References

Details

(Whiteboard: [nicer][qa-])

Attachments

(1 file, 2 obsolete files)

c:\tools\msvs10\vc\include\errno.h(102) : error C2220: warning treated as error - no 'object' file generated c:\tools\msvs10\vc\include\errno.h(102) : warning C4005: 'EHOSTUNREACH' : macro redefinition e:\builds\moz2_slave\try-w32-d-00000000000000000000\build\media\mtransport\third_party\nrappkit\src\port\win32\include\csi_platform.h(82) : see previous definition of 'EHOSTUNREACH' Didn't fail locally :(
Attached patch bug896704_nrappkit (obsolete) — Splinter Review
Assignee: nobody → VYV03354
Attachment #779396 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #833389 - Flags: review?(rjesup)
Comment on attachment 833389 [details] [diff] [review] bug896704_nrappkit Forwarding review to abr (Adam, you can decide if ekr needs to see this; I don't think so if you're good with it). Note: don't use splinter to review this patch; it gets confused by the patch format. Download or perhaps just view in Details.
Attachment #833389 - Flags: review?(rjesup) → review?(adam)
Comment on attachment 833389 [details] [diff] [review] bug896704_nrappkit Review of attachment 833389 [details] [diff] [review]: ----------------------------------------------------------------- r- for unconditional removal of a compatibility feature of nrappkit. I'm adding Ted to review moz.build, since build system changes need a build peer review. ::: media/mtransport/third_party/nrappkit/src/port/win32/include/csi_platform.h @@ +76,5 @@ > #define NR_SOCKET_READ(sock,buf,count) recv((sock),(buf),(count),0) > #define NR_SOCKET_WRITE(sock,buf,count) send((sock),(buf),(count),0) > #define NR_SOCKET_CLOSE(sock) closesocket(sock) > #endif > I suspect this serves a purpose on some platforms, and would expect that nrappkit wants to maintain compatibility with that platform. To that end, we really need the change to be more like: #ifndef EHOSTUNREACH #define EHOSTUNREACH WSAEHOSTUNREACH #endif You may need to add a "#include <errno.h>" before this set of statements to ensure that errno.h has a chance to define this macro if it's going to.
Attachment #833389 - Flags: review?(ted)
Attachment #833389 - Flags: review?(adam)
Attachment #833389 - Flags: review-
Comment on attachment 833389 [details] [diff] [review] bug896704_nrappkit Review of attachment 833389 [details] [diff] [review]: ----------------------------------------------------------------- Your review as module peer is fine for trivial moz.build changes like this.
Attachment #833389 - Flags: review?(ted)
Attached patch patch v2Splinter Review
Attachment #833389 - Attachment is obsolete: true
Attachment #8342337 - Flags: review?(adam)
Comment on attachment 8342337 [details] [diff] [review] patch v2 Review of attachment 8342337 [details] [diff] [review]: ----------------------------------------------------------------- Looks good to me.
Attachment #8342337 - Flags: review?(adam) → review+
Whiteboard: [nicer]
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
Whiteboard: [nicer] → [nicer][qa-]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: