743404 - nsHtml5TreeOperation.cpp:276:28: warning: comparison of unsigned expression >= 0 is always true [-Wtype-limits]

Status: RESOLVED FIXED

(Core :: DOM: HTML Parser, defect)

Description

[Daniel Holbert [:dholbert]]

g++ 4.6.3 gives me this build warning:
{
parser/html/nsHtml5TreeOperation.cpp: In member function ‘nsresult nsHtml5TreeOperation::Perform(nsHtml5TreeOpExecutor*, nsIContent**)’:

parser/html/nsHtml5TreeOperation.cpp:276:28: warning: comparison of unsigned expression >= 0 is always true [-Wtype-limits]
}
...for this code:
> 275         PRUint32 pos = parent->IndexOf(node);
> 276         NS_ASSERTION((pos >= 0), "Element not found as child of its parent");

The assertion there probably wants to compare to nsTArray::NoIndex (which is something like UINT_MAX, IIRC)

This code has been basically the same since this file was added in bug 487949, in 2009, but I think GCC only recently started reporting warnings for this issue.

Comment 1

[Daniel Holbert [:dholbert]]

(In reply to Daniel Holbert [:dholbert] from comment #0)
> The assertion there probably wants to compare to nsTArray::NoIndex (which is
> something like UINT_MAX, IIRC)
> 
> This code has been basically the same since this file was added in bug
> 487949, in 2009, but I think GCC only recently started reporting warnings
> for this issue.

Oh, nevermind -- I was assuming parent was a nsTArray, but it's actually it's a nsIContent*.  And I seem to remember nsIContent::IndexOf recently changing to return an unsigned value, so that would make sense as a proximate cause for this warning cropping up.

Comment 2

[Daniel Holbert [:dholbert]]

er, just kidding -- nsINode::IndexOf returns a _signed_ value and always has, AFAICT:
http://hg.mozilla.org/mozilla-central/annotate/a402c2064466/content/base/public/nsINode.h#l439

So the real bug here is that |pos| is a PRUint32 when it should be a PRInt32.

Comment 3

[Daniel Holbert [:dholbert]]

Attachment: fix

Comment 4

[Henri Sivonen (:hsivonen)]

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

Thanks for catching this.

Comment 5

[Daniel Holbert [:dholbert]]

No problem -- thanks for the review!

Pushed: https://hg.mozilla.org/integration/mozilla-inbound/rev/c5a10dbe276b

Comment 6

[Matt Brubeck (:mbrubeck)]

https://hg.mozilla.org/mozilla-central/rev/c5a10dbe276b