Closed
Bug 1257317
Opened 9 years ago
Closed 3 years ago
media\webrtc\trunk\webrtc/base/criticalsection.h(59): warning C4312: 'reinterpret_cast': conversion from 'DWORD' to 'HANDLE' of greater size from dom/media/systemservices
Categories
(Core :: WebRTC, defect, P3)
Core
WebRTC
Tracking
()
People
(Reporter: gps, Unassigned)
References
Details
Attachments
(1 obsolete file)
This warning is being unmasked in bug 1124033 and causes Visual Studio 2015 Update 1 builds to fail in automation due to warnings as errors.
Reporter | ||
Comment 1•9 years ago
|
||
As part of unblocking building with VS2015u1 in automation, I'm mass
disabling compiler warnings that are turned into errors. This is not
the preferred mechanism to fix compilation warnings. So hopefully
this patch never lands because someone insists on fixing the underlying
problem instead. But if it does land, hopefully the workaround is
only temporary. That being said, this warning appears to occur in 3rd
party code. So our hands may be tied.
Review commit: https://reviewboard.mozilla.org/r/40557/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/40557/
Attachment #8731406 -
Flags: review?(gsquelart)
Throwing it over the webrtc fence.
Assignee: nobody → gps
Component: Audio/Video → WebRTC
Comment on attachment 8731406 [details]
MozReview Request: Bug 1257317 - Disable C4312 to unblock compilation on VS2015; r?jesup
Randell, I think you should take over this review. Thanks.
(Can't see a way to change this in ReviewBoard.)
Attachment #8731406 -
Flags: review?(gsquelart) → review?(rjesup)
Reporter | ||
Comment 4•9 years ago
|
||
Comment on attachment 8731406 [details]
MozReview Request: Bug 1257317 - Disable C4312 to unblock compilation on VS2015; r?jesup
Review request updated; see interdiff: https://reviewboard.mozilla.org/r/40557/diff/1-2/
Attachment #8731406 -
Attachment description: MozReview Request: Bug 1257317 - Disable C4312 to unblock compilation on VS2015; r?gerald → MozReview Request: Bug 1257317 - Disable C4312 to unblock compilation on VS2015; r?jesup
Reporter | ||
Comment 6•9 years ago
|
||
Comment on attachment 8731406 [details]
MozReview Request: Bug 1257317 - Disable C4312 to unblock compilation on VS2015; r?jesup
Review request updated; see interdiff: https://reviewboard.mozilla.org/r/40557/diff/2-3/
Comment 7•9 years ago
|
||
Comment on attachment 8731406 [details]
MozReview Request: Bug 1257317 - Disable C4312 to unblock compilation on VS2015; r?jesup
https://reviewboard.mozilla.org/r/40557/#review37233
I really shouldn't r+ changes to the ipc moz.build file...
even systemservices in dom/media is outside my direct purview as webrtc module owner, but I'm willing to r+ that.
Remove ipc, and r+ for the rest - but if you hope this won't land, file bugs for the warnings (and un-dup the criticalsection.h warning bug) so there's something to tell us what to fix. Is it too much to hope for a "(bug blah)" in the build files where these are inserted?
Attachment #8731406 -
Flags: review?(rjesup) → review+
Updated•9 years ago
|
Rank: 25
Priority: -- → P2
Reporter | ||
Updated•9 years ago
|
Attachment #8731406 -
Attachment is obsolete: true
Reporter | ||
Comment 8•9 years ago
|
||
I'm landing the disabling of C4312 in bug 1124033. Repurposing this bug to track fixing the underlying warning.
Reporter | ||
Updated•9 years ago
|
Assignee: gps → nobody
Blocks: 1259293
Paul, I've got a fix for this, but I'm not sure whether it should be applied directly or through some patching system (to help with future updating from webrtc.org).
Changeset: https://hg.mozilla.org/try/rev/c0d888378c623c8f987a365af18157f8eb6709c8
From this try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=0c77bd3299bb22629aa103e4aa6172f4e84631c4
Note, this was also fixed on webrtc.org in Oct 2015:
https://chromium.googlesource.com/external/webrtc/+/a10492ff33152cef5a835761125219d3e4ee21d0%5E!/#F0
So if you are going to bring it here anytime soon, we can wait for that instead.
Flags: needinfo?(pkerr)
Comment 10•9 years ago
|
||
Gerald, we are currently pulling in webrtc.org stable branch 49 into our codebase and should pick up the google fix in the process.
Flags: needinfo?(pkerr)
Thank you. I'm assuming bug 1250356 tracks this work.
Depends on: 1250356
Comment 12•7 years ago
|
||
Mass change P2->P3 to align with new Mozilla triage process.
Priority: P2 → P3
Updated•5 years ago
|
Assignee: nobody → nobody
Fixed in Firefox 53 by bug 1250356.
Status: NEW → RESOLVED
Closed: 3 years ago
status-firefox92:
--- → fixed
status-firefox93:
--- → fixed
status-firefox94:
--- → fixed
status-firefox-esr78:
--- → fixed
status-firefox-esr91:
--- → fixed
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•