Closed Bug 299560 Opened 15 years ago Closed 15 years ago
CSSParser .cpp:935 Comparison between signed and unsigned integer expressions
> sub_end = aBuffer.FindChar(PRUnichar(','), sub); sub_end is unsigned; however, FindChar() may return -1. > if (sub_end == -1) is therefore always false.
Why would it be false? In both cases the -1 is coerced to unsigned before the comparison is done...
(In reply to comment #2) > Why would it be false? In both cases the -1 is coerced to unsigned before the > comparison is done... Perhaps I should have said, it would "technically speaking" be false. But at least fixing this will shut the compiler up.. :)
Comment on attachment 188125 [details] [diff] [review] fix If you want to fix the warning, use nsSubstring::index_type(kNotFound) instead of -1. FindChar really *should* be returning index_type. And then run Ian's testcases (see the bug for which this code was checked in).
(Really, all the |PRUint32|s should be |nsSubstring::index_type|s, so PRUint32(kNotFound) is fine too.)
(In reply to comment #5) > (Really, all the |PRUint32|s should be |nsSubstring::index_type|s, so > PRUint32(kNotFound) is fine too.) So you're suggesting an explicit conversion of -1 to zero, right? Shouldn't we account for the case when FindChar actually returns zero, though?
Why to zero? It'd convert to PR_UNIT32_MAX, basically...
You're right, of course.. I wrongly assumed -1 would be cast to 0.
Attachment #188125 - Attachment is obsolete: true
Comment on attachment 188133 [details] [diff] [review] another fix (In reply to comment #4) > And then run Ian's > testcases (see the bug for which this code was checked in). Firefox with this patch applied passes all of Hixie's tests.
Attachment #188133 - Flags: approval1.8b4? → approval1.8b4+
Checked in by timeless (2005-07-13 11:16).
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.