Last Comment Bug 743404 - nsHtml5TreeOperation.cpp:276:28: warning: comparison of unsigned expression >= 0 is always true [-Wtype-limits]
: nsHtml5TreeOperation.cpp:276:28: warning: comparison of unsigned expression >...
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: HTML: Parser (show other bugs)
: Trunk
: x86_64 Linux
: -- normal (vote)
: mozilla14
Assigned To: Daniel Holbert [:dholbert]
:
: Andrew Overholt [:overholt]
Mentors:
Depends on:
Blocks: buildwarning html5-parsing-land
  Show dependency treegraph
 
Reported: 2012-04-06 18:00 PDT by Daniel Holbert [:dholbert]
Modified: 2012-04-11 09:04 PDT (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
fix (1.21 KB, patch)
2012-04-06 18:08 PDT, Daniel Holbert [:dholbert]
hsivonen: review+
Details | Diff | Splinter Review

Description Daniel Holbert [:dholbert] 2012-04-06 18:00:08 PDT
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] 2012-04-06 18:03:04 PDT
(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] 2012-04-06 18:05:03 PDT
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] 2012-04-06 18:08:03 PDT
Created attachment 613048 [details] [diff] [review]
fix
Comment 4 Henri Sivonen (:hsivonen) 2012-04-10 01:20:00 PDT
Comment on attachment 613048 [details] [diff] [review]
fix

Thanks for catching this.
Comment 5 Daniel Holbert [:dholbert] 2012-04-10 10:46:54 PDT
No problem -- thanks for the review!

Pushed: https://hg.mozilla.org/integration/mozilla-inbound/rev/c5a10dbe276b
Comment 6 Matt Brubeck (:mbrubeck) 2012-04-11 09:04:55 PDT
https://hg.mozilla.org/mozilla-central/rev/c5a10dbe276b

Note You need to log in before you can comment on or make changes to this bug.