Closed Bug 947736 Opened 6 years ago Closed 6 years ago

Build modules/libpref/ in unified mode

Categories

(Core :: General, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla29

People

(Reporter: ehsan, Assigned: ehsan)

References

Details

Attachments

(1 file, 1 obsolete file)

No description provided.
Assignee: nobody → ehsan
Blocks: unified
Attachment #8344374 - Flags: review?(benjamin)
Comment on attachment 8344374 [details] [diff] [review]
Build modules/libpref/ in unified mode

What's up with nsPrefBranch::NotifyObserver returning an nsresult-as-int? That seems weird and there's no comment explaining it.
Attachment #8344374 - Flags: review?(benjamin) → review-
(In reply to Benjamin Smedberg  [:bsmedberg] from comment #2)
> Comment on attachment 8344374 [details] [diff] [review]
> Build modules/libpref/ in unified mode
> 
> What's up with nsPrefBranch::NotifyObserver returning an nsresult-as-int?
> That seems weird and there's no comment explaining it.

I'm glad you asked.  :-)

We use that function as a callback argument.  We define the type of that argument in two places, one with an nsresult return code, the other with an int return code:

http://mxr.mozilla.org/mozilla-central/source/modules/libpref/src/prefapi.h#157
http://mxr.mozilla.org/mozilla-central/source/modules/libpref/public/Preferences.h#29

To make things more fun, as far as I can tell the return code is never used: <http://mxr.mozilla.org/mozilla-central/source/modules/libpref/src/Preferences.cpp#164>.  But that doesn't really matter, since we always return NS_OK.  So I just had to pick something and move on, hence the current patch.

If you don't want to accept this patch then it's fine, but please suggest an alternative.  Should I make this return void?
Flags: needinfo?(benjamin)
Yes, returning void seems better than living with conflicting typedefs.
Flags: needinfo?(benjamin)
Attachment #8344374 - Attachment is obsolete: true
Attachment #8345413 - Flags: review?(benjamin)
Attachment #8345413 - Flags: review?(benjamin) → review+
I had to edit all of the places which defined a pref callback function.

https://hg.mozilla.org/integration/mozilla-inbound/rev/98be0e9cbc15
https://hg.mozilla.org/mozilla-central/rev/98be0e9cbc15
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
You need to log in before you can comment on or make changes to this bug.