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)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla9

People

(Reporter: mwu, Assigned: mwu)

References

Details

Attachments

(1 file, 1 obsolete file)

Attached patch Cleanup PRBool abuses in prefapi.cpp (obsolete) (deleted) — 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 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-
(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.
Attachment #545601 - Attachment is obsolete: true
Attachment #549001 - Flags: review?
Attachment #549001 - Flags: review? → review?(benjamin)
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+
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.
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]
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.

Attachment

General

Created:
Updated:
Size: