Closed Bug 1210463 Opened 5 years ago Closed 5 years ago

Misc. cleanups in ModuleUtils.h

Categories

(Core :: XPCOM, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla44
Tracking Status
firefox44 --- fixed

People

(Reporter: mccr8, Assigned: mccr8)

Details

Attachments

(3 files)

No description provided.
I just started up a try run, but these are pretty simple changes. It builds and runs locally.

try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=51504d003922
Attachment #8668541 - Flags: review?(nfroyd)
Assignee: nobody → continuation
Note that I intentionally left the OOM null check in NS_GENERIC_FACTORY_SINGLETON_CONSTRUCTOR, because the getter could do anything.
Attachment #8668541 - Flags: review?(nfroyd) → review+
Comment on attachment 8668542 [details] [diff] [review]
part 2 - Use some smart pointers in ModuleUtils.h.

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

::: xpcom/components/ModuleUtils.h
@@ +29,2 @@
>                                                                                \
>      return rv;                                                                \

Bonus points if you just return the result of the QI call here.

@@ -79,5 @@
>      if (nullptr == inst) {                                                    \
>          rv = NS_ERROR_OUT_OF_MEMORY;                                          \
>          return rv;                                                            \
>      }                                                                         \
> -    /* NS_ADDREF(inst); */                                                    \

Commented out reference counting is the best.

@@ +80,2 @@
>                                                                                \
>      return rv;                                                                \

Same thing here.
Attachment #8668542 - Flags: review?(nfroyd) → review+
Attachment #8668543 - Flags: review?(nfroyd) → review+
(In reply to Nathan Froyd [:froydnj] from comment #5)
> Bonus points if you just return the result of the QI call here.
Why is that okay to do? What guarantees that _InstanceClass and aIID are related?
(In reply to Andrew McCreight [:mccr8] from comment #6)
> (In reply to Nathan Froyd [:froydnj] from comment #5)
> > Bonus points if you just return the result of the QI call here.
> Why is that okay to do? What guarantees that _InstanceClass and aIID are
> related?

Oh sorry, I misread that! Never mind.
(In reply to Nathan Froyd [:froydnj] from comment #5)
> Bonus points if you just return the result of the QI call here.

I did this.

> >          rv = NS_ERROR_OUT_OF_MEMORY;                                          \
> >          return rv;                                                            \

I also changed a few places like the above to return the constant directly, and eliminated some declarations of rv when they were no longer used.
Hopefully the dt1 timeouts in my try run at not due to my patches. I can't imagine how, but you never know.
You need to log in before you can comment on or make changes to this bug.