Closed Bug 1037363 Opened 5 years ago Closed 5 years ago
--enable-webrtc build broken on BSDs: rtp
_receiver _video .cc:289:69: error: 'ntohs' was not declared in this scope
In file included from media/webrtc/trunk/webrtc/modules/modules_rtp_rtcp/Unified_cpp_modules_rtp_rtcp1.cpp:54: media/webrtc/trunk/webrtc/modules/rtp_rtcp/source/rtp_receiver_video.cc:289:24: error: use of undeclared identifier 'ntohs' payload_length = ntohs(*(reinterpret_cast<uint16_t*>(payload))); ^ 1 error generated.
ntohs() is brought in by sys/types.h.. i'm surprised it's not in the include tree..
https://tbpl.mozilla.org/?tree=Try&rev=2e925cd06390 POSIX says <arpa/inet.h> should provide ntohs() and htons(). The manpage for Linux, OS X, Solaris and NetBSD recommend <arpa/inet.h>. FreeBSD and DragonFly also allow to use <netinet/in.h> instead. OpenBSD went with <sys/types.h> included by almost any header. However, the actual provision may differ e.g., NetBSD pulls htons/ntohs from <sys/endian.h> via <sys/types.h> like OpenBSD. As the code using ntohs() and htons() is unconditional #else is better here. And non-Windows systems are supposed to be POSIX compliant (to some extent).  http://pubs.opengroup.org/onlinepubs/9699919799/functions/htonl.html  http://man7.org/linux/man-pages/man3/htons.3.html
Attachment #8454368 - Flags: review?(rjesup)
Comment on attachment 8454368 [details] [diff] [review] fix Review of attachment 8454368 [details] [diff] [review]: ----------------------------------------------------------------- Got seduced by comments in the webrtc.org test code (#include <netinet/in.h> // for htons, htonl, etc). Thanks
Attachment #8454368 - Flags: review?(rjesup) → review+
Assignee: nobody → jbeich
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla33
Comment on attachment 8454368 [details] [diff] [review] fix It's so easy to miss when blockee is backported. Only noticed because the buildbot turned red. http://buildbot.rhaalovely.net/builders/mozilla-aurora-freebsd-amd64/builds/360/steps/build/logs/stdio Approval Request Comment [Feature/regressing bug #]: regression from bug 1036049 [User impact if declined]: build broken on at least FreeBSD unless --disable-webrtc [Describe test coverage new/current, TBPL]: landed on m-c [Risks and why]: Low, broken build at most. [String/UUID change made/needed]: None
Attachment #8454368 - Flags: approval-mozilla-aurora?
Attachment #8454368 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
You need to log in before you can comment on or make changes to this bug.