Closed Bug 788792 Opened 7 years ago Closed 7 years ago

GCC 4.6+ build warnings in nsNavHistoryResult.cpp for "unsigned expression >= 0 is always true" [-Wtype-limits]

Categories

(Toolkit :: Places, defect)

x86_64
Linux
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla18

People

(Reporter: dholbert, Assigned: ehsan)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 1 obsolete file)

I filed bug 757960 on a bunch of build warnings in nsNavHistoryResult.cpp.  It looks like the patch that landed there fixed some of them, but not all of them -- or maybe new ones were introduced since then.  Either way, I have these warnings in my current m-c build:
{
../../../../mozilla/toolkit/components/places/nsNavHistoryResult.cpp: In member function ‘nsresult nsNavHistoryContainerResultNode::ReverseUpdateStats(int32_t)’:
../../../../mozilla/toolkit/components/places/nsNavHistoryResult.cpp:695:30: warning: comparison of unsigned expression >= 0 is always true [-Wtype-limits]
../../../../mozilla/toolkit/components/places/nsNavHistoryResult.cpp:696:23: warning: comparison of unsigned expression >= 0 is always true [-Wtype-limits]
../../../../mozilla/toolkit/components/places/nsNavHistoryResult.cpp: In member function ‘bool nsNavHistoryContainerResultNode::DoesChildNeedResorting(uint32_t, nsNavHistoryContainerResultNode::SortComparator, const char*)’:
../../../../mozilla/toolkit/components/places/nsNavHistoryResult.cpp:883:24: warning: comparison of unsigned expression >= 0 is always true [-Wtype-limits]
../../../../mozilla/toolkit/components/places/nsNavHistoryResult.cpp: In member function ‘nsresult nsNavHistoryContainerResultNode::RemoveChildAt(int32_t, bool)’:
../../../../mozilla/toolkit/components/places/nsNavHistoryResult.cpp:1630:32: warning: comparison of unsigned expression >= 0 is always true [-Wtype-limits]
../../../../mozilla/toolkit/components/places/nsNavHistoryResult.cpp: In member function ‘virtual nsresult nsNavHistoryFolderResultNode::OnItemMoved(int64_t, int64_t, int32_t, int64_t, int32_t, uint16_t, const nsACString_internal&, const nsACString_internal&, const nsACString_internal&)’:
../../../../mozilla/toolkit/components/places/nsNavHistoryResult.cpp:4027:25: warning: comparison of unsigned expression >= 0 is always true [-Wtype-limits]
../../../../mozilla/toolkit/components/places/nsNavHistoryResult.cpp:4032:18: warning: comparison of unsigned expression >= 0 is always true [-Wtype-limits]
}
Attached patch Patch (v1) (obsolete) — Splinter Review
Assignee: nobody → ehsan
Status: NEW → ASSIGNED
Attachment #667270 - Flags: review?(mak77)
Comment on attachment 667270 [details] [diff] [review]
Patch (v1)

Review of attachment 667270 [details] [diff] [review]:
-----------------------------------------------------------------

FindChild returns a signed integer, we have been dumb enough to assign it to an unsigned... (ensureItemPosition indeed wants an unsigned, but an explicit cast should be done when passing the value to it...)
Attachment #667270 - Flags: review?(mak77) → review-
the second removal instead is correct, o the problem is only the first one that comes from FindChild()
Attached patch Patch (v2)Splinter Review
Indeed.  Nice catch.
Attachment #667270 - Attachment is obsolete: true
Attachment #667632 - Flags: review?(mak77)
Comment on attachment 667632 [details] [diff] [review]
Patch (v2)

Review of attachment 667632 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/components/places/nsNavHistoryResult.cpp
@@ +695,3 @@
>        NS_ASSERTION(ourIndex >= 0, "Could not find self in parent");
>        if (ourIndex >= 0)
>          EnsureItemPosition(ourIndex);

please add a static_cast here, just to be really explicit
Attachment #667632 - Flags: review?(mak77) → review+
https://hg.mozilla.org/mozilla-central/rev/267d712aabdd
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla18
You need to log in before you can comment on or make changes to this bug.