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

RESOLVED FIXED in mozilla14

Status

()

Core
HTML: Parser
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: dholbert, Assigned: dholbert)

Tracking

(Blocks: 1 bug)

Trunk
mozilla14
x86_64
Linux
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

5 years ago
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.
(Assignee)

Comment 1

5 years ago
(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.
No longer blocks: 487949
(Assignee)

Comment 2

5 years ago
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.
(Assignee)

Comment 3

5 years ago
Created attachment 613048 [details] [diff] [review]
fix
Assignee: nobody → dholbert
Status: NEW → ASSIGNED
Attachment #613048 - Flags: review?(hsivonen)
(Assignee)

Updated

5 years ago
Blocks: 487949, 187528
Comment on attachment 613048 [details] [diff] [review]
fix

Thanks for catching this.
Attachment #613048 - Flags: review?(hsivonen) → review+
(Assignee)

Comment 5

5 years ago
No problem -- thanks for the review!

Pushed: https://hg.mozilla.org/integration/mozilla-inbound/rev/c5a10dbe276b
Target Milestone: --- → mozilla14
https://hg.mozilla.org/mozilla-central/rev/c5a10dbe276b
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.