Closed Bug 1001708 Opened 6 years ago Closed 6 years ago

Link error due to statics used with ternary operator

Categories

(Core :: WebRTC: Audio/Video, defect)

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla31

People

(Reporter: garvan, Assigned: garvan)

References

Details

Attachments

(1 file, 1 obsolete file)

src/media/webrtc/signaling/src/common/YuvStamper.cpp:357: error: undefined reference to 'mozilla::YuvStamper::sYOn'
src/media/webrtc/signaling/src/common/YuvStamper.cpp:357: error: undefined reference to 'mozilla::YuvStamper::sYOff'
src/media/webrtc/signaling/src/common/YuvStamper.cpp:457: error: undefined reference to 'mozilla::YuvStamper::sLumaMin'
src/media/webrtc/signaling/src/common/YuvStamper.cpp:457: error: undefined reference to 'mozilla::YuvStamper::sLumaMax'

Getting a link error due to statics used with the ternary operator when building fennec 

http://stackoverflow.com/questions/5446005/why-dont-static-member-variables-play-well-with-the-ternary-operator
Attached patch 1001708bug.diff (obsolete) — Splinter Review
Following the stack overflow suggestion, this code links
Given a choice, I would refactor these code segments to use if statements and drop the ternary operator.
Questions: why are you getting a link error on this when we are not?  What compiler version are you using?  Have you followed the prerequisites on the MDN pages for building Fennec on your machine?  What version of the SDK/NDK are you using?  What's the mozconfig?
(In reply to Paul Kerr [:pkerr] from comment #2)
> Given a choice, I would refactor these code segments to use if statements
> and drop the ternary operator.

I agree, the +foo : +bar usage is overly tricky even if it works.  But first let's understand why the reporter is having this problem when we are not.
Been building daily for the past week, broke with today's update.
Using android-17 sdk (in ~/ADT_SDK in the following).

More info:

$ uname -a
Darwin gkeeley13311 13.1.0 Darwin Kernel Version 13.1.0: Thu Jan 16 19:40:37 PST 2014; root:xnu-2422.90.20~2/RELEASE_X86_64 x86_64

$ clang -v
Apple LLVM version 5.1 (clang-503.0.40) (based on LLVM 3.4svn)
Target: x86_64-apple-darwin13.1.0
Thread model: posix

export PATH=$PATH:$HOME/ADT_SDK/tools:$HOME/ADT_SDK/build-tools:$HOME/ADT_SDK/platform-tools

— mozconfig —

export PATH="`brew --prefix ccache`/libexec:$PATH"
mk_add_options MOZ_OBJDIR=@TOPSRCDIR@/obj-fennec
mk_add_options MOZ_MAKE_FLAGS="-s -j4"
ac_add_options --enable-debug
ac_add_options --enable-debug-symbols
ac_add_options --disable-optimize
mk_add_options AUTOCONF=/usr/local/Cellar/autoconf213/2.13/bin/autoconf213
ac_add_options --with-android-ndk="/Applications/adt-mac/android-ndk-r8e"
ac_add_options --with-android-sdk="/Users/gkeeley/ADT_SDK/platforms/android-17"
ac_add_options --enable-application=mobile/android
ac_add_options --target=arm-linux-androideabi
ac_add_options --with-ccache
Looks sane.  I wonder if the host OS being Mac instead of Linux is the issue.  In any case, we should avoid tripping over this.  Please redo the patch with if's (and add a comment), and mark for review by me.  Thanks!
Flags: needinfo?(gkeeley)
Attached patch 1001708bug.diffSplinter Review
(In reply to Randell Jesup [:jesup] from comment #6)
Switched to if-conditional and added comment so it won't get switched back to ternary in future.
Attachment #8413031 - Attachment is obsolete: true
Attachment #8413316 - Flags: review?(rjesup)
Flags: needinfo?(gkeeley)
Attachment #8413316 - Flags: review?(rjesup) → review+
https://hg.mozilla.org/mozilla-central/rev/f2203bec9507
Assignee: nobody → gkeeley
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.