Closed
Bug 1002729
Opened 10 years ago
Closed 10 years ago
Link failure due to static const integers in WebRTC
Categories
(Core :: WebRTC, defect)
Tracking
()
RESOLVED
FIXED
mozilla32
People
(Reporter: mrbkap, Assigned: mrbkap)
References
Details
Attachments
(1 file, 1 obsolete file)
1.52 KB,
patch
|
jib
:
review+
|
Details | Diff | Splinter Review |
Compiler: * g++-4.7.real (Ubuntu/Linaro 4.7.2-2ubuntu1) 4.7.2 * GNU gold (GNU Binutils for Ubuntu 2.22.90.20120924) 1.11 Build options: --enable-debug --disable-optimize 0:57.00 /usr/bin/ld.gold.real: error: read-only segment has dynamic relocations 0:57.00 /home/mrbkap/work/main/mozilla/content/media/webrtc/MediaEngine.h:158: error: undefined reference to 'mozilla::MediaEngine::DEFAULT_169_VIDEO_WIDTH' 0:57.00 /home/mrbkap/work/main/mozilla/content/media/webrtc/MediaEngine.h:158: error: undefined reference to 'mozilla::MediaEngine::DEFAULT_43_VIDEO_WIDTH' 0:57.00 /home/mrbkap/work/main/mozilla/content/media/webrtc/MediaEngine.h:163: error: undefined reference to 'mozilla::MediaEngine::DEFAULT_169_VIDEO_HEIGHT' 0:57.00 /home/mrbkap/work/main/mozilla/content/media/webrtc/MediaEngine.h:163: error: undefined reference to 'mozilla::MediaEngine::DEFAULT_43_VIDEO_HEIGHT' This appears to be a pretty common problem: static const integers with no definition (only a declaration in the class) being used in ternary expressions.
Assignee | ||
Comment 1•10 years ago
|
||
Attachment #8414017 -
Flags: review?(rjesup)
Comment 2•10 years ago
|
||
Comment on attachment 8414017 [details] [diff] [review] Patch Review of attachment 8414017 [details] [diff] [review]: ----------------------------------------------------------------- ::: content/media/webrtc/MediaEngine.h @@ +154,5 @@ > } > private: > static int32_t GetDefWidth(bool aHD = false) { > + return aHD ? +MediaEngine::DEFAULT_169_VIDEO_WIDTH : > + +MediaEngine::DEFAULT_43_VIDEO_WIDTH; This is confusing to the reader (and someone may try to undo it). Add a comment as in the other bug 1007xxx, and I suggest if (aHD) { return MediaEngine::DEFAULT_169_VIDEO_WIDTH; } return MediaEngine::DEFAULT_43_VIDEO_WIDTH; with normal brace styling, of course. If so, r+ @@ +159,5 @@ > } > > static int32_t GetDefHeight(bool aHD = false) { > + return aHD ? +MediaEngine::DEFAULT_169_VIDEO_HEIGHT : > + +MediaEngine::DEFAULT_43_VIDEO_HEIGHT; ditto
Attachment #8414017 -
Flags: review?(rjesup) → review+
Comment 3•10 years ago
|
||
FWIW the '+' trick is used here http://stackoverflow.com/questions/5446005/why-dont-static-member-variables-play-well-with-the-ternary-operator In either case I think this warrants a comment. This seems to be a problem with GCC (arguably). See http://gcc.gnu.org/bugzilla/show_bug.cgi?id=13795#c3
Comment 5•10 years ago
|
||
Note that GCC 4.8.2 (Fedora 19) does not show this problem, and we don't see it on the builders. All the reports recently have been for Ubuntu running 4.7. glandium: we're fixing this, but is this something we should be avoiding, or should we be telling people to update GCC? Just to verify for the future.
Flags: needinfo?(mh+mozilla)
Comment 6•10 years ago
|
||
The instance of this problem that I reported in bug 1002844 is with LLVM 5.1, so this is not GCC-specific.
Comment 7•10 years ago
|
||
> glandium: we're fixing this, but is this something we should be avoiding, or
> should we be telling people to update GCC? Just to verify for the future.
The former.
Flags: needinfo?(mh+mozilla)
Assignee | ||
Comment 8•10 years ago
|
||
Attachment #8414017 -
Attachment is obsolete: true
Assignee | ||
Updated•10 years ago
|
Keywords: checkin-needed
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → mrbkap
Comment 10•10 years ago
|
||
Comment on attachment 8415534 [details] [diff] [review] For checkin Review of attachment 8415534 [details] [diff] [review]: ----------------------------------------------------------------- r=me + forward from jesup
Attachment #8415534 -
Flags: review+
Comment 11•10 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/4f77407d1839
Keywords: checkin-needed
Target Milestone: --- → mozilla32
Comment 12•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/4f77407d1839
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•