Closed Bug 1150437 Opened 10 years ago Closed 10 years ago

[EME] Relicense gmp-clearkey to only depend on Apache2 code

Categories

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

39 Branch
x86_64
Windows 8.1
defect

Tracking

()

RESOLVED FIXED
mozilla40
Tracking Status
firefox39 --- wontfix
firefox40 --- fixed

People

(Reporter: cpearce, Assigned: cpearce)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

To make it easier for partners to reuse parts of our code in proprietary plugins, gmp-clearkey should only contain and use/#include Apache 2 licensed code.
Attached patch Patch (deleted) — Splinter Review
Relicense MPL to Apache 2 in gmp-clearkey, and remove dependency on MFBT. Will take the first review I get... https://treeherder.mozilla.org/#/jobs?repo=try&revision=72ce6f4fee6f
Attachment #8587317 - Flags: review?(jwwang)
Attachment #8587317 - Flags: review?(edwin)
Attachment #8587317 - Flags: review?(ajones)
Comment on attachment 8587317 [details] [diff] [review] Patch Review of attachment 8587317 [details] [diff] [review]: ----------------------------------------------------------------- Define NDEBUG for release builds
Attachment #8587317 - Flags: review?(edwin) → review+
Attachment #8587317 - Flags: review?(ajones) → review+
Attachment #8587317 - Flags: review?(jwwang) → review+
We should fix this soon, but we don't need to uplift the license change.
Priority: -- → P1
(In reply to Edwin Flores [:eflores] [:edwin] from comment #5) > Comment on attachment 8587317 [details] [diff] [review] > Patch > > Review of attachment 8587317 [details] [diff] [review]: > ----------------------------------------------------------------- > > Define NDEBUG for release builds I tested, and we do define NDEBUG in --disable-debug builds and don't in --enable-debug. So we're good as is.
Comment on attachment 8587317 [details] [diff] [review] Patch Review of attachment 8587317 [details] [diff] [review]: ----------------------------------------------------------------- ::: media/gmp-clearkey/0.1/RefCounted.h @@ +57,5 @@ > + T* operator->() const { return mPtr; } > + > +private: > + void Assign(T* aPtr) { > + if (mPtr) { There's a foot-gun here. If we assign a RefPtr to itself, we'll we'll deref mPtr before we addref it, so we'll end up destroying the thing pointed to if that's the last ref.
Comment on attachment 8587317 [details] [diff] [review] Patch Review of attachment 8587317 [details] [diff] [review]: ----------------------------------------------------------------- ::: media/gmp-clearkey/0.1/RefCounted.h @@ +57,5 @@ > + T* operator->() const { return mPtr; } > + > +private: > + void Assign(T* aPtr) { > + if (mPtr) { I noticed and concluded it was safe because Assign() is private and there is no assign operator and RefPtr is not convertible to T*. What call path could result in self-assignment?
Status: NEW → RESOLVED
Closed: 10 years ago
Flags: in-testsuite-
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: