| Summary: | [JSC] Add exception checks in JSStringBuilder and Array#join | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Yusuke Suzuki <ysuzuki> | ||||||
| Component: | New Bugs | Assignee: | Yusuke Suzuki <ysuzuki> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | ews-watchlist, keith_miller, mark.lam, msaboff, saam, tzagallo, webkit-bug-importer | ||||||
| Priority: | P2 | Keywords: | InRadar | ||||||
| Version: | WebKit Nightly Build | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| Attachments: |
|
||||||||
|
Description
Yusuke Suzuki
2020-07-02 19:51:11 PDT
Created attachment 403434 [details]
Patch
Comment on attachment 403434 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=403434&action=review > Source/JavaScriptCore/runtime/JSStringJoiner.h:169 > + return; do we really need this? Comment on attachment 403434 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=403434&action=review >> Source/JavaScriptCore/runtime/JSStringJoiner.h:169 >> + return; > > do we really need this? Since we call `scope.release()`, returning here is better I think. Even if we add a new code after this if branch, then it should work well :) Comment on attachment 403434 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=403434&action=review >>> Source/JavaScriptCore/runtime/JSStringJoiner.h:169 >>> + return; >> >> do we really need this? > > Since we call `scope.release()`, returning here is better I think. Even if we add a new code after this if branch, then it should work well :) Could you have done `RELEASE_AND_RETURN(append(jsString->viewWithUnderlyingString(globalObject)));` instead? Comment on attachment 403434 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=403434&action=review >>>> Source/JavaScriptCore/runtime/JSStringJoiner.h:169 >>>> + return; >>> >>> do we really need this? >> >> Since we call `scope.release()`, returning here is better I think. Even if we add a new code after this if branch, then it should work well :) > > Could you have done `RELEASE_AND_RETURN(append(jsString->viewWithUnderlyingString(globalObject)));` instead? Changed. Created attachment 403441 [details]
Patch
Committed r263889: <https://trac.webkit.org/changeset/263889> All reviewed patches have been landed. Closing bug and clearing flags on attachment 403441 [details]. |