Closed Bug 1322863 Opened 5 years ago Closed 5 years ago

use UniquePtr for managing pointers where possible.

Categories

(Core :: Gecko Profiler, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla53
Tracking Status
firefox53 --- fixed

People

(Reporter: froydnj, Assigned: froydnj)

References

Details

Attachments

(3 files)

I probably won't catch everything, but I have a patch series to fix a goodly number of places.
Depends on: 1142197
Smart pointers are better than raw pointers.
Attachment #8823387 - Flags: review?(mstange)
Smart pointers are better than raw pointers.

I think I like this patch, but there is also some weirdness that ThreadInfo owns
ThreadProfile objects, and ThreadProfile objects can own ThreadInfo objects
(though apparently not at the same time?).  We should sort that out.
Attachment #8823388 - Flags: review?(mstange)
Smart pointers are better than raw pointers, and this makes clients of
PlatformData slightly simpler because they don't have to manage
destruction themselves: the new UniquePtr-derived type handles all of
that for us.
Attachment #8823389 - Flags: review?(mstange)
Attachment #8823387 - Flags: review?(mstange) → review+
Comment on attachment 8823388 [details] [diff] [review]
make ThreadInfo manage mProfile with UniquePtr

Review of attachment 8823388 [details] [diff] [review]:
-----------------------------------------------------------------

I don't know about the the ThreadInfo vs ThreadProfile ownership situation, but this patch seems like it should be safe either way.
Attachment #8823388 - Flags: review?(mstange) → review+
Comment on attachment 8823389 [details] [diff] [review]
manage Sampler::PlatformData with UniquePtr

Review of attachment 8823389 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good.
It still feels somewhat unnecessary complex - we're abstracting something out that's known at compile time, right?
It may also be unused, because I don't see any callers of GetProfiledThread.
Attachment #8823389 - Flags: review?(mstange) → review+
Pushed by nfroyd@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/41e0e99837f9
part 1 - make ThreadInfo::mName a UniqueFreePtr; r=mstange
https://hg.mozilla.org/integration/mozilla-inbound/rev/07f764d10078
part 2 - make ThreadInfo manage mProfile with UniquePtr; r=mstange
https://hg.mozilla.org/integration/mozilla-inbound/rev/e1a7c28606f1
part 3 - manage Sampler::PlatformData with UniquePtr; r=mstange
You need to log in before you can comment on or make changes to this bug.