2052560 - Remove always-true clause from NS_ASSERTION in nsNavBookmarks::AdjustIndices (Coverity CID 1274453)

Status: RESOLVED FIXED

(Toolkit :: Places, task)

Description

[Sylvestre Ledru [:Sylvestre]]

Filing as a good first bug to learn workflows.

In toolkit/components/places/nsNavBookmarks.cpp, nsNavBookmarks::AdjustIndices contains an assertion clause that Coverity flagged as a dead/tautological guard (CID 1274453).

The assertion is:

    NS_ASSERTION(
        aStartIndex >= 0 && aEndIndex <= INT32_MAX && aStartIndex <= aEndIndex,
        "Bad indices");

The aEndIndex <= INT32_MAX clause is always true: aEndIndex is an int32_t, whose maximum possible value is exactly INT32_MAX, so the comparison can never be false.

The fix is to drop that clause, leaving the two meaningful invariants:

NS_ASSERTION(aStartIndex >= 0 && aStartIndex <= aEndIndex, "Bad indices");

This is behavior-preserving: the removed clause never contributed to the assertion result.

Link to the code:
https://searchfox.org/mozilla-central/source/toolkit/components/places/nsNavBookmarks.cpp

To verify the fix, the tree should still build:

./mach build binaries

Tutorial to contribute:
https://firefox-source-docs.mozilla.org/contributing/contribution_quickref.html
https://firefox-source-docs.mozilla.org/contributing/stack_quickref.html

Please don't ask for the bug to be assigned. It will be automatically assigned to the first patch.

Comment 1

[Tim McNulty]

Attachment: Bug 2052560 - Remove always-true clause from NS_ASSERTION in nsNavBookmarks::AdjustIndices r?sylvestre

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/a7ce8ff119d0
https://hg.mozilla.org/integration/autoland/rev/4338e62b4858
Remove always-true clause from NS_ASSERTION in nsNavBookmarks::AdjustIndices r=sylvestre,places-reviewers,Standard8

Comment 3

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/4338e62b4858