Closed
Bug 671190
Opened 13 years ago
Closed 13 years ago
Stop using BOGUS_DEFAULT_*_PREF_VALUE in prefapi.cpp and cleanup PRBool abuse
Categories
(Core :: Preferences: Backend, defect)
Core
Preferences: Backend
Tracking
()
RESOLVED
FIXED
mozilla9
People
(Reporter: mwu, Assigned: mwu)
References
Details
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
benjamin
:
review+
|
Details | Diff | Splinter Review |
This gets rid of BOGUS_DEFAULT_*_PREF_VALUE and adds a new flag called PREF_HAS_DEFAULT. It also changes PREF_SetBoolPref to accept PRInt32 because that's what its callers have always passed.
As a side-effect, this fix allows us to use the number -5632 in default int prefs now.
Attachment #545601 -
Flags: review?(benjamin)
Comment 1•13 years ago
|
||
Comment on attachment 545601 [details] [diff] [review]
Cleanup PRBool abuses in prefapi.cpp
I don't understand why you switch setboolpref to PRInt32. If anything, we should start using `bool`, but PRBool is fine for now and will let the static analysis notice if we start passing non-bool values.
Attachment #545601 -
Flags: review?(benjamin) → review-
Assignee | ||
Comment 2•13 years ago
|
||
(In reply to comment #1)
> I don't understand why you switch setboolpref to PRInt32. If anything, we
> should start using `bool`, but PRBool is fine for now and will let the
> static analysis notice if we start passing non-bool values.
It's actually precisely because static analysis found that users were passing non-bool. We can correct the user instead.
Assignee | ||
Comment 3•13 years ago
|
||
Attachment #545601 -
Attachment is obsolete: true
Attachment #549001 -
Flags: review?
Assignee | ||
Updated•13 years ago
|
Attachment #549001 -
Flags: review? → review?(benjamin)
Comment 4•13 years ago
|
||
Comment on attachment 549001 [details] [diff] [review]
Cleanup PRBool abuses in prefapi.cpp, v2
It totally blows my mind that .setBoolPref took a long. Here's hoping that nobody actually used that quirk!
Attachment #549001 -
Flags: review?(benjamin) → review+
Comment 5•13 years ago
|
||
I had a patch somewhat similar to this in one of my old trees somewhere.
It differed slightly in that it simply stored the default pref as the current value of the user pref, so that getting the pref didn't have to do the check.
Assignee | ||
Comment 6•13 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/0fba4d8f69c5
After bug 343800 , the pref code started forcing all values coming through setBoolPref to be either PR_TRUE or PR_FALSE so there shouldn't be any code now that depends on storing ints in bool prefs.
Whiteboard: [inbound]
Comment 7•13 years ago
|
||
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Whiteboard: [inbound]
Target Milestone: --- → mozilla9
You need to log in
before you can comment on or make changes to this bug.
Description
•