Closed
Bug 1287756
Opened 8 years ago
Closed 8 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•8 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•8 years ago
|
Attachment #8772389 -
Flags: review?(dholbert) → review?(bent.mozilla)
Assignee | ||
Updated•8 years ago
|
Component: Layout → DOM: IndexedDB
Comment 2•8 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•8 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•8 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•8 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•8 years ago
|
||
bugherder |
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
You need to log in
before you can comment on or make changes to this bug.
Description
•