Closed Bug 899693 Opened 6 years ago Closed 6 years ago

Fix unsafe references near HashableValue::setValue

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla25

People

(Reporter: terrence, Assigned: terrence)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

We can easily make this a HandleValue.
Attachment #783261 - Flags: review?(sphink)
Comment on attachment 783261 [details] [diff] [review]
ur_MapObjectSetValue-v0.diff

Review of attachment 783261 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/jsiter.h
@@ +299,5 @@
>          ok = ok && Next(cx, iterator, &currentValue);
>          return ok && !currentValue.get().isMagic(JS_NO_ITER_VALUE);
>      }
>  
> +    MutableHandleValue value() {

Why is this a *Mutable*HandleValue? When would you want to change the current value of the iterator? I don't see it used in this patch; am I missing it?

Actually, this doesn't seem like it belongs in this patch at all. It would make more sense in the other patch. Ooh... is it needed there?
Comment on attachment 783261 [details] [diff] [review]
ur_MapObjectSetValue-v0.diff

Review of attachment 783261 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/jsiter.h
@@ +299,5 @@
>          ok = ok && Next(cx, iterator, &currentValue);
>          return ok && !currentValue.get().isMagic(JS_NO_ITER_VALUE);
>      }
>  
> +    MutableHandleValue value() {

Oh! It was a non-const reference, so the caller can modify the Value at the current point of iteration. Duh.
Attachment #783261 - Flags: review?(sphink) → review+
https://hg.mozilla.org/mozilla-central/rev/1d49d7996875
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla25
You need to log in before you can comment on or make changes to this bug.