Closed Bug 1251881 Opened 4 years ago Closed 4 years ago

use UniquePtr instead of ScopedDeletePtr in mozglue/linker/

Categories

(Core :: mozglue, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla47
Tracking Status
firefox47 --- fixed

People

(Reporter: froydnj, Assigned: froydnj)

Details

Attachments

(1 file)

There are two instances:

- MappableSeekableZStream::Create uses ScopedDeletePtr, which is
  actually a little odd, since MappableSeekableZStream, as a Mappable,
  is refcounted, and it's unusual to use two different kinds of smart
  pointers for a single class.  I think the more natural thing would be
  for MappableSeekableZStream::Create to return an already_AddRefed, but
  that change spirals out of the realm of "simple refactoring" quickly.

- SzipCompress::run uses ScopedDeletePtr along with some dubious use of
  raw pointers.  I changed things out for UniquePtr and also tidied the
  memory management a bit, which I think makes things a little clearer.
Comment on attachment 8724455 [details] [diff] [review]
use UniquePtr instead of ScopedDeletePtr in mozglue/linker/

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

::: mozglue/linker/Mappable.cpp
@@ +377,5 @@
>  MappableSeekableZStream::Create(const char *name, Zip *zip,
>                                  Zip::Stream *stream)
>  {
>    MOZ_ASSERT(stream->GetType() == Zip::Stream::STORE);
> +  UniquePtr<MappableSeekableZStream> mappable(new MappableSeekableZStream(zip));

Couldn't that work with a RefPtr, which would make more sense? FWIW, the use of ScopedDeletePtr (and AutoDeletePtr before that) here predates Mappables being refcounted...
Attachment #8724455 - Flags: review?(mh+mozilla) → review+
https://hg.mozilla.org/mozilla-central/rev/af6363aa513b
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
You need to log in before you can comment on or make changes to this bug.