[webkit-reviews] review granted: [Bug 197535] [GLib] Expose typed arrays in the public API : [Attachment 454538] Patch

[bugzilla-daemon at webkit.org]

Carlos Garcia Campos <cgarcia at igalia.com> has granted Adrian Perez
<aperez at igalia.com>'s request for review:
Bug 197535: [GLib] Expose typed arrays in the public API
https://bugs.webkit.org/show_bug.cgi?id=197535

Attachment 454538: Patch

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




--- Comment #24 from Carlos Garcia Campos <cgarcia at igalia.com> ---
Comment on attachment 454538
  --> https://bugs.webkit.org/attachment.cgi?id=454538
Patch

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

> Source/JavaScriptCore/API/glib/JSCValue.cpp:1748
> +    g_return_val_if_fail(JSC_IS_CONTEXT(context), nullptr);
> +

g_return_val_if_fail(type != JSC_TYPED_ARRAY_NONE, nullptr);

> Source/JavaScriptCore/API/glib/JSCValue.cpp:1772
> + * The @type must *not* be #JSC_TYPED_ARRAY_NONE.

#JSC_TYPED_ARRAY_NONE -> %JSC_TYPED_ARRAY_NONE

> Source/JavaScriptCore/API/glib/JSCValue.cpp:1790
> +    g_return_val_if_fail(JSC_IS_VALUE(arrayBuffer), nullptr);
> +    g_return_val_if_fail(jsc_value_is_array_buffer(arrayBuffer), nullptr);
> +    g_return_val_if_fail(type != JSC_TYPED_ARRAY_NONE, nullptr);
> +

We explicitly say that -1 is used for omitting length, so I would add
g_return_val_if_fail(length >= -1, nullptr);

> Source/JavaScriptCore/API/glib/JSCValue.cpp:1849
> +    using namespace JSC;

Adding JSC:: is not that much.

> Source/JavaScriptCore/API/glib/JSCValue.cpp:1901
> + * ##JSCTypedArrayType), and has the `offset` over the underlying array

##JSCTypedArrayType -> #JSCTypedArrayType

> Source/JavaScriptCore/API/glib/JSCValue.cpp:1934
> +    auto* ptr = JSObjectGetTypedArrayBytesPtr(jsContext, jsObject,
&exception);

prt -> bytes