Closed Bug 785060 Opened 7 years ago Closed 7 years ago

Check for JS_NewUCStringCopyZ failure

Categories

(Core :: DOM: Core & HTML, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla20

People

(Reporter: Ms2ger, Assigned: Infinity)

Details

Attachments

(1 file, 3 obsolete files)

Of the places we call JS_NewUCStringCopyZ outside js/, only one out of four deals with the exception correctly.
Attached patch Patchv1 (obsolete) — Splinter Review
Does this look fine?
Attachment #682703 - Flags: feedback?(Ms2ger)
Comment on attachment 682703 [details] [diff] [review]
Patchv1

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

LGTM.

::: content/html/content/src/nsHTMLMediaElement.cpp
@@ +1468,5 @@
> +    NS_WARNING("Failed to perform string copy");
> +    args->error = true;
> +    return PL_DHASH_STOP;
> +  }
> +  

Just remove this trailing whitespace.
Attachment #682703 - Flags: feedback?(Ms2ger) → review+
Assignee: nobody → junky.argonaut
Ok...done! Do you want me to run any specific tests before I submit the patch for review?
Attached patch Patchv2 (obsolete) — Splinter Review
It compiled faster than expected...wow!
Attachment #682703 - Attachment is obsolete: true
Attachment #682727 - Flags: review?(Ms2ger)
Comment on attachment 682727 [details] [diff] [review]
Patchv2

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

r=me
Attachment #682727 - Flags: review?(Ms2ger) → review+
Could you submit a patch with a proper author and commit message? That would make it easier to commit it for you. See https://developer.mozilla.org/en-US/docs/Mercurial_FAQ#How_can_I_generate_a_patch_for_somebody_else_to_check-in_for_me.3F for more details.
Attached patch Patchv3 (obsolete) — Splinter Review
I had it configured exactly like that. Check it out!
http://pastebin.mozilla.org/1949277
So I shouldn't be doing hg diff -p -U 8 I guess.....anyways edited patch here :)
Attachment #682727 - Attachment is obsolete: true
Attachment #683643 - Flags: review?(Ms2ger)
Could you also make sure there is a commit message in the patch? Maybe something like

Bug 785060 - Check for allocation failure in JS_NewUCStringCopyZ calls; r=Ms2ger
Attached patch patchv4Splinter Review
Hope its ok now. I got MQ fixed just now. So any extra metadata you'd like, lemme know.
Attachment #683643 - Attachment is obsolete: true
Attachment #683643 - Flags: review?(Ms2ger)
Attachment #685258 - Flags: review?(Ms2ger)
Comment on attachment 685258 [details] [diff] [review]
patchv4

Thanks!
Attachment #685258 - Flags: review?(Ms2ger) → review+
Keywords: checkin-needed
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/bab7ae06e579
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla20
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.