Closed
Bug 824648
Opened 11 years ago
Closed 11 years ago
VMFunctions.cpp:367:5: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
FIXED
mozilla20
People
(Reporter: dholbert, Assigned: dholbert)
References
(Blocks 1 open bug)
Details
Attachments
(1 file)
772 bytes,
patch
|
nbp
:
review+
|
Details | Diff | Splinter Review |
Build warning: VMFunctions.cpp:367:5: warning: comparison between signed and unsigned integer expressions [-Wsign-compare] For this code: { > 364 bool > 365 CharCodeAt(JSContext *cx, HandleString str, int32_t index, uint32_t *code) > 366 { > 367 JS_ASSERT(index < str->length()); [...] > 373 *code = chars[index]; } Build warning is because index is signed, whereas JSString::length() is unsigned (a size_t): https://mxr.mozilla.org/mozilla-central/source/js/src/vm/String.h#256 We probably want to (a) assert that index is nonnegative, and then (b) cast it to a uint32_t before comparing it to length(). Or we could change the CharCodeAt function-signature -- that might be more correct -- but it looks like that'd be more invasive.
Assignee | ||
Comment 1•11 years ago
|
||
oops, meant to link to the chunk of code quoted in comment 0. That MXR link is: https://mxr.mozilla.org/mozilla-central/source/js/src/ion/VMFunctions.cpp#364
Comment 2•11 years ago
|
||
(In reply to Daniel Holbert [:dholbert] from comment #0) > Or we could change the CharCodeAt function-signature -- that might be more > correct -- but it looks like that'd be more invasive. Changing the function signature might cause inconsistency with the register allocation of IonMonkey. Asserting that we have no-negative value should be fine because we are supposed to have bounds check before any call to CharCodeAt.
Assignee | ||
Comment 3•11 years ago
|
||
Assignee: general → dholbert
Status: NEW → ASSIGNED
Attachment #695764 -
Flags: review?(nicolas.b.pierron)
Updated•11 years ago
|
Attachment #695764 -
Flags: review?(nicolas.b.pierron) → review+
Assignee | ||
Comment 4•11 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/47a6822f5e3f
Assignee | ||
Comment 5•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/47a6822f5e3f
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla20
You need to log in
before you can comment on or make changes to this bug.
Description
•