[webkit-reviews] review granted: [Bug 190836] [BigInt] JSBigInt::createWithLength should throw when length is greater than JSBigInt::maxLength : [Attachment 354604] Patch

[bugzilla-daemon at webkit.org]

Saam Barati <sbarati at apple.com> has granted Caio Lima <ticaiolima at gmail.com>'s
request for review:
Bug 190836: [BigInt] JSBigInt::createWithLength should throw when length is
greater than JSBigInt::maxLength
https://bugs.webkit.org/show_bug.cgi?id=190836

Attachment 354604: Patch

https://bugs.webkit.org/attachment.cgi?id=354604&action=review




--- Comment #14 from Saam Barati <sbarati at apple.com> ---
Comment on attachment 354604
  --> https://bugs.webkit.org/attachment.cgi?id=354604
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=354604&action=review

r=me w/ comments.

> Source/JavaScriptCore/runtime/JSBigInt.cpp:112
> +    RELEASE_ASSERT(bigInt);

No need for this RELEASE_ASSERT. This will never return null. If you can't
allocate the cell, it'll crash. It could return nullptr if you used
tryAllocateCell.

> Source/JavaScriptCore/runtime/JSBigInt.cpp:419
> +	   result = absoluteAddOne(exec, result, SignOption::Signed);
> +	   RETURN_IF_EXCEPTION(scope, nullptr);
> +	   return result;

This could be:
scope.release()
return absoluteAddOne(exec, result, SignOption::Signed)

> Source/JavaScriptCore/runtime/JSBigInt.cpp:498
> +    result = absoluteAddOne(exec, result, SignOption::Signed);
> +    RETURN_IF_EXCEPTION(scope, nullptr);
> +    return result;

this could be:
scope.release();
return absoluteAddOne(exec, result, SignOption::Signed);

> Source/JavaScriptCore/runtime/Operations.cpp:75
> +	       JSBigInt* result = JSBigInt::add(callFrame,
WTF::get<JSBigInt*>(leftNumeric), WTF::get<JSBigInt*>(rightNumeric));
> +	       RETURN_IF_EXCEPTION(scope, { });
> +	       return result;

ditto w/ scope.release() before calling add

> Source/JavaScriptCore/runtime/Operations.h:354
> +	       JSBigInt* result = JSBigInt::sub(exec,
WTF::get<JSBigInt*>(leftNumeric), WTF::get<JSBigInt*>(rightNumeric));
> +	       RETURN_IF_EXCEPTION(scope, { });
> +	       return result;

ditto

> Source/JavaScriptCore/runtime/Operations.h:377
> +	       JSBigInt* result = JSBigInt::multiply(state,
WTF::get<JSBigInt*>(leftNumeric), WTF::get<JSBigInt*>(rightNumeric));
> +	       RETURN_IF_EXCEPTION(scope, { });
> +	       return result;

ditto