Closed
Bug 1287756
Opened 5 years ago
Closed 5 years ago
[Static Analysis][Assignment in Assert] In function NoteFinishedMaintenance
Categories
(Core :: Storage: IndexedDB, defect)
Core
Storage: IndexedDB
Tracking
()
RESOLVED
FIXED
mozilla50
Tracking | Status | |
---|---|---|
firefox50 | --- | fixed |
People
(Reporter: andi, Assigned: andi)
References
Details
Attachments
(1 file)
Our own static analysis tool based on clang triggered that an assignment is present inside an assert expression: >> AssertIsOnBackgroundThread(); >> MOZ_ASSERT(aMaintenance); >> MOZ_ASSERT(mCurrentMaintenance = aMaintenance); >> >> mCurrentMaintenance = nullptr; >> ProcessMaintenanceQueue(); I think the correct behaviour here would have been to have a logical expression instead of an assignment, like: >> MOZ_ASSERT(mCurrentMaintenance == aMaintenance);
Assignee | ||
Comment 1•5 years ago
|
||
Review commit: https://reviewboard.mozilla.org/r/65216/diff/#index_header See other reviews: https://reviewboard.mozilla.org/r/65216/
Attachment #8772389 -
Flags: review?(dholbert)
Assignee | ||
Updated•5 years ago
|
Attachment #8772389 -
Flags: review?(dholbert) → review?(bent.mozilla)
Assignee | ||
Updated•5 years ago
|
Component: Layout → DOM: IndexedDB
Comment 2•5 years ago
|
||
hg blame says that Jan Varga added this line, reviewed by khuey: https://hg.mozilla.org/mozilla-central/rev/ca8e9cc4f661#l1.108 One of them would be the most appropriate reviewer, probably. I believe bent is largely inactive, as noted by his bugzilla handle.
Flags: needinfo?(bpostelnicu)
Assignee | ||
Comment 3•5 years ago
|
||
Comment on attachment 8772389 [details] Bug 1287756 - prevent assignment in assert expression for |mCurrentMaintenance|. Review request updated; see interdiff: https://reviewboard.mozilla.org/r/65216/diff/1-2/
Attachment #8772389 -
Flags: review?(bent.mozilla) → review?(jvarga)
Assignee | ||
Comment 4•5 years ago
|
||
(In reply to Daniel Holbert [:dholbert] from comment #2) > hg blame says that Jan Varga added this line, reviewed by khuey: > https://hg.mozilla.org/mozilla-central/rev/ca8e9cc4f661#l1.108 > > One of them would be the most appropriate reviewer, probably. I believe > bent is largely inactive, as noted by his bugzilla handle. I've forwarded this to Jan
Flags: needinfo?(bpostelnicu)
Comment 5•5 years ago
|
||
Comment on attachment 8772389 [details] Bug 1287756 - prevent assignment in assert expression for |mCurrentMaintenance|. https://reviewboard.mozilla.org/r/65216/#review62352 r=me, Thanks!
Attachment #8772389 -
Flags: review?(jvarga) → review+
Pushed by bpostelnicu@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/99837fd92258 prevent assignment in assert expression for |mCurrentMaintenance|.r=janv
Comment 7•5 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/99837fd92258
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
You need to log in
before you can comment on or make changes to this bug.
Description
•