Closed
Bug 947736
Opened 11 years ago
Closed 11 years ago
Build modules/libpref/ in unified mode
Categories
(Core :: General, defect)
Core
General
Tracking
()
RESOLVED
FIXED
mozilla29
People
(Reporter: ehsan.akhgari, Assigned: ehsan.akhgari)
References
Details
Attachments
(1 file, 1 obsolete file)
11.15 KB,
patch
|
benjamin
:
review+
|
Details | Diff | Splinter Review |
No description provided.
Assignee | ||
Comment 1•11 years ago
|
||
Assignee | ||
Updated•11 years ago
|
Attachment #8344374 -
Flags: review?(benjamin)
Comment 2•11 years ago
|
||
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-
Assignee | ||
Comment 3•11 years ago
|
||
(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)
Comment 4•11 years ago
|
||
Yes, returning void seems better than living with conflicting typedefs.
Flags: needinfo?(benjamin)
Assignee | ||
Updated•11 years ago
|
Attachment #8344374 -
Attachment is obsolete: true
Assignee | ||
Comment 5•11 years ago
|
||
Assignee | ||
Updated•11 years ago
|
Attachment #8345413 -
Flags: review?(benjamin)
Updated•11 years ago
|
Attachment #8345413 -
Flags: review?(benjamin) → review+
Assignee | ||
Comment 6•11 years ago
|
||
Assignee | ||
Comment 7•11 years ago
|
||
Backed out because of bustage: https://hg.mozilla.org/integration/mozilla-inbound/rev/982196bdc263
Assignee | ||
Comment 8•11 years ago
|
||
I had to edit all of the places which defined a pref callback function.
https://hg.mozilla.org/integration/mozilla-inbound/rev/98be0e9cbc15
Comment 9•11 years ago
|
||
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla29
You need to log in
before you can comment on or make changes to this bug.
Description
•