671190 - Stop using BOGUS_DEFAULT_*_PREF_VALUE in prefapi.cpp and cleanup PRBool abuse

Status: RESOLVED FIXED

(Core :: Preferences: Backend, defect)

Description

[Michael Wu [:mwu]]

Attachment: 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]

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]]

(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]]

Attachment: Cleanup PRBool abuses in prefapi.cpp, v2

Comment 4

[Benjamin Smedberg]

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

[[e-mail]]

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]]

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)]

http://hg.mozilla.org/mozilla-central/rev/0fba4d8f69c5