Remove the MSVC exemption for FAIL_ON_WARNINGS in media/mtransport

RESOLVED FIXED in mozilla28

Status

()

defect
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: emk, Assigned: emk)

Tracking

unspecified
mozilla28
x86_64
Windows 8
Points:
---
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [nicer][qa-])

Attachments

(1 attachment, 2 obsolete attachments)

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 :(
Posted 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)
Posted 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]
https://hg.mozilla.org/mozilla-central/rev/ea36b327a1ee
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
Whiteboard: [nicer] → [nicer][qa-]
Duplicate of this bug: 836500
You need to log in before you can comment on or make changes to this bug.