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)
Tracking
()
RESOLVED
FIXED
mozilla28
People
(Reporter: emk, Assigned: emk)
References
Details
(Whiteboard: [nicer][qa-])
Attachments
(1 file, 2 obsolete files)
17.68 KB,
patch
|
abr
:
review+
|
Details | Diff | Splinter Review |
Assignee | ||
Comment 1•12 years ago
|
||
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 :(
Assignee | ||
Comment 2•12 years ago
|
||
Assignee: nobody → VYV03354
Attachment #779396 -
Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #833389 -
Flags: review?(rjesup)
Assignee | ||
Comment 3•12 years ago
|
||
Comment 4•12 years ago
|
||
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 5•12 years ago
|
||
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 6•12 years ago
|
||
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)
Assignee | ||
Comment 7•12 years ago
|
||
Attachment #833389 -
Attachment is obsolete: true
Attachment #8342337 -
Flags: review?(adam)
Comment 8•12 years ago
|
||
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+
Updated•12 years ago
|
Whiteboard: [nicer]
Assignee | ||
Comment 9•12 years ago
|
||
Flags: in-testsuite-
Comment 10•12 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
Updated•11 years ago
|
Whiteboard: [nicer] → [nicer][qa-]
You need to log in
before you can comment on or make changes to this bug.
Description
•