Last Comment Bug 671190 - Stop using BOGUS_DEFAULT_*_PREF_VALUE in prefapi.cpp and cleanup PRBool abuse
: Stop using BOGUS_DEFAULT_*_PREF_VALUE in prefapi.cpp and cleanup PRBool abuse
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: Preferences: Backend (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla9
Assigned To: Michael Wu [:mwu]
:
Mentors:
Depends on: 669808
Blocks:
  Show dependency treegraph
 
Reported: 2011-07-12 23:27 PDT by Michael Wu [:mwu]
Modified: 2011-08-23 01:42 PDT (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Cleanup PRBool abuses in prefapi.cpp (6.26 KB, patch)
2011-07-12 23:27 PDT, Michael Wu [:mwu]
benjamin: review-
Details | Diff | Splinter Review
Cleanup PRBool abuses in prefapi.cpp, v2 (8.62 KB, patch)
2011-07-27 18:22 PDT, Michael Wu [:mwu]
benjamin: review+
Details | Diff | Splinter Review

Description Michael Wu [:mwu] 2011-07-12 23:27:41 PDT
Created attachment 545601 [details] [diff] [review]
Cleanup PRBool abuses in prefapi.cpp

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.
Comment 1 Benjamin Smedberg AWAY UNTIL 2-AUG-2016 [:bsmedberg] 2011-07-26 12:06:16 PDT
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.
Comment 2 Michael Wu [:mwu] 2011-07-26 20:45:43 PDT
(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.
Comment 3 Michael Wu [:mwu] 2011-07-27 18:22:08 PDT
Created attachment 549001 [details] [diff] [review]
Cleanup PRBool abuses in prefapi.cpp, v2
Comment 4 Benjamin Smedberg AWAY UNTIL 2-AUG-2016 [:bsmedberg] 2011-08-15 07:52:55 PDT
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!
Comment 5 neil@parkwaycc.co.uk 2011-08-15 13:17:25 PDT
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.
Comment 6 Michael Wu [:mwu] 2011-08-22 19:24:44 PDT
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.
Comment 7 Mounir Lamouri (:mounir) 2011-08-23 01:42:21 PDT
http://hg.mozilla.org/mozilla-central/rev/0fba4d8f69c5

Note You need to log in before you can comment on or make changes to this bug.