Closed Bug 837395 Opened 7 years ago Closed 7 years ago
Windows build fails in nrappkit under VC9 because EAFNOSUPPORT is undefined
This is a regression from bug 825927, but I can't set it as such because that is a security bug. The actual compiler error is: c:/Users/wag/mozilla/mozilla2/fx-obj/media/mtransport/third_party/nrappkit/nrappkit_nrappkit/../../../../../../media/mtransport/third_party/nrappkit/src/util/util.c(576) : error C2065: 'EAFNOSUPPORT' : undeclared identifier
This allows the build to complete, although probably not the correct approach.
Comment on attachment 709378 [details] [diff] [review] workaround Review of attachment 709378 [details] [diff] [review]: ----------------------------------------------------------------- ::: media/mtransport/third_party/nrappkit/src/util/util.c @@ +513,5 @@ > #include <sys/socket.h> > #endif > +#ifndef EAFNOSUPPORT > +#define EAFNOSUPPORT WSAEAFNOSUPPORT > +#endif Please move this inside the #ifdef WIN32
Attachment #709378 - Flags: review+
Seems reasonable to me; you could check with ted or a build peer, or commit
Comment on attachment 709378 [details] [diff] [review] workaround I do think this is as close we're going to get to ”the right approach” as we can. In a quick survey of how other projects have solved this problem, substituting WSA* equivalents for missing errno codes seems to be the most common approach. The actual errno codes returned from inet_ntop are specified by POSIX, so we're not really at liberty to select a different code. The one tweak I would suggest is moving the "#ifndef ENOAFSUPPORT" block inside the "#ifdef WIN32" block, as it won't ever apply to other builds. (I was in the process of R+'ing this, but it looks like Jesup beat me to it)
Ideally we should have a header file somewhere central where we can do something like this: http://mxr.mozilla.org/mozilla-central/source/netwerk/sctp/src/netinet/sctp_os_userspace.h#139
(In reply to Philip Chee from comment #5) > Ideally we should have a header file somewhere central where we can do > something like this: > http://mxr.mozilla.org/mozilla-central/source/netwerk/sctp/src/netinet/ > sctp_os_userspace.h#139 The problem is that this is part of nrappkit, which is 3rd party code. We upstream all our changes back to the original project, so relying on code elsewhere in the Mozilla tree isn't feasible.
Actually, it is probably better to put this inside the #ifdef win32.
OIC you already said move it inside the ifdef. I will post a check-in ready patch.
Assignee: nobody → bill
Status: NEW → ASSIGNED
Comment on attachment 709394 [details] [diff] [review] check-in ready patch This version looks ready for checkin.
Attachment #709394 - Flags: review+
(In reply to Adam Roach [:abr] from comment #10) > Comment on attachment 709394 [details] [diff] [review] > check-in ready patch > > This version looks ready for checkin. Great that makes it clearer for those who do the checkin-needed bugs. I don't have checkin priv's.
Comment on attachment 709394 [details] [diff] [review] check-in ready patch We've mostly been having Randell do our check-ins. I've tagged him with a checkin request on this patch.
Attachment #709394 - Flags: checkin?(rjesup)
I'll get it committed later today
Target Milestone: --- → mozilla21
Attachment #709394 - Flags: checkin?(rjesup) → checkin+
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Whiteboard: [WebRTC], [blocking-webrtc+] → [WebRTC], [blocking-webrtc+], [qa-]
You need to log in before you can comment on or make changes to this bug.