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

Categories

(Core :: WebRTC: Networking, defect)

All
FreeBSD
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla33
Tracking Status
firefox32 --- fixed
firefox33 --- fixed

People

(Reporter: jbeich, Assigned: jbeich)

References

Details

Attachments

(1 file)

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..
Attached patch fixSplinter Review
https://tbpl.mozilla.org/?tree=Try&rev=2e925cd06390

POSIX says[1] <arpa/inet.h> should provide ntohs() and htons(). The manpage for Linux[2], 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).

[1] http://pubs.opengroup.org/onlinepubs/9699919799/functions/htonl.html
[2] 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+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/673ad6aa248d
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.