Closed
Bug 1424030
Opened 7 years ago
Closed 7 years ago
Fix integer overflow of places.database.lastMaintenance in testing/profiles/prefs_general.js
Categories
(Toolkit :: Places, defect, P2)
Toolkit
Places
Tracking
()
RESOLVED
FIXED
mozilla59
| Tracking | Status | |
|---|---|---|
| firefox59 | --- | fixed |
People
(Reporter: n.nethercote, Assigned: n.nethercote)
References
Details
Attachments
(1 file, 1 obsolete file)
|
(deleted),
patch
|
standard8
:
review+
|
Details | Diff | Splinter Review |
Bug 1382444 added this line to testing/profiles/prefs_general.js:
> user_pref("places.database.lastMaintenance", 7258114800);
(7258114800 seconds after 1970 is the start of the year 2200.)
libpref stores integers prefs as int32_t and the current parser doesn't detect
overflow. So this overflows to -1331819792. (I detected this with the new prefs
parser from bug 1423840, which does detect integer overflow.) As a result the
condition testing this pref in
toolkit/components/places/PlacesCategoriesStarter.js ends up always succeeding
in tests, which is the exact opposite of what was intended.
This patch changes it to 2147483647 (the year 2038), the maximum int32_t value.
MozReview-Commit-ID: LJQmMqQ9hFL
|
Assignee | |
Comment 1•7 years ago
|
||
Another possibility would be to simply remove this user_pref, since it's
clearly not doing what it was supposed to...
Attachment #8935494 -
Flags: review?(standard8)
|
||
Comment 2•7 years ago
|
||
The scope of the pref is to avoid intermittents and useless tinderbox work, so it would probably do what is supposed to do, when correct :)
|
|
Assignee | |
Comment 3•7 years ago
|
||
The signal from bug 1382444 is unclear -- it seems like the intermittents that prompted the change went away even though the change was bogus. But I'm happy to switch to a non-bogus value if you think it'll help :)
Blocks: 1382444
|
||
Comment 4•7 years ago
|
||
Comment on attachment 8935494 [details] [diff] [review]
Fix integer overflow of places.database.lastMaintenance in testing/profiles/prefs_general.js
Review of attachment 8935494 [details] [diff] [review]:
-----------------------------------------------------------------
I suspect I was confusing milliseconds and seconds again... I think we should fix this anyway, and keep the value. I've believe idle-daily is now turned off fully and so this wouldn't kick in anyway, but just in case ;-)
Attachment #8935494 -
Flags: review?(standard8) → review+
|
|
||
Updated•7 years ago
|
Priority: -- → P2
|
|
Assignee | |
Comment 5•7 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/baa80d498effd0665b0aafd21714f3b7f0965e10
Bug 1424030 - Fix integer overflow of places.database.lastMaintenance in testing/profiles/prefs_general.js. r=standard8.
|
||
Comment 6•7 years ago
|
||
| bugherder | ||
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
status-firefox59:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla59
| Comment hidden (mozreview-request) |
|
|
Assignee | |
Comment 8•7 years ago
|
||
Comment on attachment 8947345 [details]
Bug 1424030 - Fix integer overflow of places.database.lastMaintenance in testing/profiles/prefs_general.js. .
Sorry, this patch was supposed to go to bug 1434813.
Attachment #8947345 -
Attachment is obsolete: true
Attachment #8947345 -
Flags: review?(mh+mozilla)
You need to log in
before you can comment on or make changes to this bug.
Description
•