Closed Bug 410929 Opened 12 years ago Closed 12 years ago

bad C++isms introduced in js/src

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: benjamin, Assigned: benjamin)

References

Details

Attachments

(1 file)

Missing casts needed for C++. I'm also getting the following warnings which look suspicious, but I'm going to ignore them for the moment:

../../../src/js/src/jsarray.cpp:1104: warning: comparison is always false due to limited range of data type
../../../src/js/src/jsarray.cpp:1206: warning: comparison is always false due to limited range of data type
Attachment #295512 - Flags: review?(crowder)
Comment on attachment 295512 [details] [diff] [review]
Cast for C++, rev. 1

Let me fetch my ruler:
01234567890123456789012345678901234567890123456789012345678901234567890123456789
+            vec = (jsval*) JS_realloc(cx, vec, 4 * (size_t) newlen * sizeof(jsval));

Should instead be (jsval*) => (jsval *), and don't spill past 80-columns:
+            vec = (jsval *) JS_realloc(cx, vec,
+                                       4 * (size_t) newlen * sizeof(jsval));

Otherwise looks good.

    if (len > (size_t) -1 / (2 * sizeof(jsval))) {
Weird.  if (len > 536870911) { ???  right?  Why isn't jsuint big enough to exceed 536870911?  What platform are you on?

The other warning is even more suspicious, we need to figure these out.  Overflows here are bad.

Thanks.  r=me with the formatting nit.
Attachment #295512 - Flags: review?(crowder) → review+
(In reply to comment #1)
>     if (len > (size_t) -1 / (2 * sizeof(jsval))) {
> Weird.  if (len > 536870911) { ???  right?  Why isn't jsuint big enough to
> exceed 536870911?  What platform are you on?

See bug 410941.

/be
Depends on: 410941
Fixed on CVS trunk.
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Flags: in-testsuite-
Flags: in-litmus-
You need to log in before you can comment on or make changes to this bug.