The default bug view has changed. See this FAQ.

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

RESOLVED FIXED in mozilla9

Status

()

Core
Preferences: Backend
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: mwu, Assigned: mwu)

Tracking

(Depends on: 1 bug)

Trunk
mozilla9
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

(Assignee)

Description

6 years ago
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.
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-
(Assignee)

Comment 2

6 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

6 years ago
Created attachment 549001 [details] [diff] [review]
Cleanup PRBool abuses in prefapi.cpp, v2
Attachment #545601 - Attachment is obsolete: true
Attachment #549001 - Flags: review?
(Assignee)

Updated

6 years ago
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+

Comment 5

6 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

6 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]
http://hg.mozilla.org/mozilla-central/rev/0fba4d8f69c5
Status: NEW → RESOLVED
Last Resolved: 6 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.