845429 - nsDocShell.cpp:3601:42: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]

Status: RESOLVED FIXED

(Core Graveyard :: DOM: Apps, defect)

Description

[Daniel Holbert [:dholbert]]

New build warning:
{
docshell/base/nsDocShell.cpp:3601:42: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
}

from this code:
3592 NS_IMETHODIMP
3593 nsDocShell::GetChildAt(int32_t aIndex, nsIDocShellTreeItem ** aChild)
3594 {
3595     NS_ENSURE_ARG_POINTER(aChild);
3596 
3597 #ifdef DEBUG
3598     if (aIndex < 0) {
3599       NS_WARNING("Negative index passed to GetChildAt");
3600     }
3601     else if (aIndex >= mChildList.Length()) {
3602       NS_WARNING("Too large an index passed to GetChildAt");
3603     }
3604 #endif
https://mxr.mozilla.org/mozilla-central/source/docshell/base/nsDocShell.cpp#3593

The warning started when we changed mChildList from a nsVoidArray (with a signed Count() method) to a nsTArray (with an unsigned Length() method), in this cset:

https://hg.mozilla.org/mozilla-central/diff/8ddc33231c6f/docshell/base/nsDocShell.cpp#l1.211

Comment 1

[Daniel Holbert [:dholbert]]

Attachment: fix v1: static_cast<uint32_t>

If we hit this clause, we're guaranteed that aIndex is non-negative.  (If it were negative, we would've entered the "if (aIndex < 0)" clause, and we wouldn't be touching this "else if" stuff.)

So, we're guaranteed that static_cast<uint32_t> will be a valid cast.  This patch adds that cast.

Comment 2

[Trevor Saunders (:tbsaunde)]

Comment on attachment 718507 [details] [diff] [review]
fix v1: static_cast<uint32_t>

>     }
>-    else if (aIndex >= mChildList.Length()) {
>+    else if (static_cast<uint32_t>(aIndex) >= mChildList.Length()) {

} else if (x) { while your here please

Comment 3

[Daniel Holbert [:dholbert]]

Sounds good, thanks!

Comment 4

[Daniel Holbert [:dholbert]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/94c4b6ba217b

Comment 5

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/94c4b6ba217b