823187 - nsSHistory.cpp:971:51: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]

Status: RESOLVED FIXED

(Core :: DOM: Navigation, defect)

Description

[Daniel Holbert [:dholbert]]

Just noticed this build warning:
{
docshell/shistory/src/nsSHistory.cpp:971:51: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
}
for this code:
> 958   int32_t startSafeIndex = NS_MAX(0, aIndex - gHistoryMaxViewers);
> 959   int32_t endSafeIndex = NS_MIN(mLength, aIndex + gHistoryMaxViewers);
> [...]
> 971   for (uint32_t i = startSafeIndex; trans && i <= endSafeIndex; i++) {
> 972     nsCOMPtr<nsIContentViewer> viewer = GetContentViewerForTransaction(trans);
> 973     safeViewers.AppendObject(viewer);
> 974     nsISHTransaction *temp = trans;
> 975     temp->GetNext(getter_AddRefs(trans));
> 976   }

We have a loop with 'i' ranging from startSafeIndex to endSafeIndex.  Those xyzSafeIndex variables are both signed, so 'i' should be too.

(It looks like we're probably pretty sure that both xyzSafeIndex variables are nonnegative -- notably, "startSafeIndex" is initted with a NS_MAX(0,...) -- so this probably doesn't actually cause problems. Still, might as well declare the loop-variable w/ a type that matches the thing it's being intitialized from & compared to.)

We've had this issue since this cset:
 https://hg.mozilla.org/mozilla-central/rev/051b3d8ed545#l15.302
for bug 683777.

Comment 1

[Daniel Holbert [:dholbert]]

Attachment: fix

This fixes it by making i an int32_t instead of uint32_t.

(Note that "i" is *only* used as a loop variable -- not as an array-index or a function-argument or anything -- so there shouldn't be any side effects from changing its type to match the others in the "for()" statement.)

Patch has 16 lines of context, to include the declarations of startSafeIndex & endSafeIndex, for clarity.

Comment 2

[Justin Lebar (not reading bugmail)]

Comment on attachment 693994 [details] [diff] [review]
fix

Thanks for the extra context!

Comment 3

[Daniel Holbert [:dholbert]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/1a490822a502

Comment 4

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/1a490822a502