On Mon, 28 Sep 2026 22:59:01 GMT, Sergey Bylokhov <[email protected]> wrote:
>> Alexander Zvegintsev has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> review comments
>
> src/java.desktop/unix/native/libawt_xawt/awt/screencast_portal.c line 127:
>
>> 125: if (!newScreens) {
>> 126: ERR("failed to allocate memory\n");
>> 127: return FALSE;
>
> Please double check that this loop actually correctly de-/allocate the data
> via g_variant_iter_loop and g_variant_unref, as of now it sounds like double
> free? And this should be handled somehow on this return as well?
>
> see: https://mail.gnome.org/archives/commits-list/2011-July/msg07600.html
> and:
>>"g_variant_iter_loop": on the first call to this function, the pointers
>>appearing on the variable argument list are assumed to point at uninitialised
>>memory. On the second and later calls, it is assumed that the same pointers
>>will be given and that they will point to the memory as set by the previous
>>call to this function. This allows the previous values to be freed, as
>>appropriate.
Thanks, `gtk->g_variant_unref(prop)` should only be called when breaking the
loop, so moved it to the allocation failure handler.
> src/java.desktop/unix/native/libawt_xawt/awt/screencast_portal.c line 887:
>
>> 885: gtk->g_variant_get(
>> 886: response,
>> 887: "(h)",
>
> the format string is wrong?
The format string is correct, but the `err` argument in `g_variant_get` is
ignored, so removed it.
`g_variant_get` doesn't report errors through `GError`.
I added `GET_VARIANT_CHECKED` macro (where applicable), which contains
`g_variant_is_of_type` safety checks for unexpected types and `g_variant_get`
call if the check is passed.
[g_unix_fd_list_get docs](https://docs.gtk.org/gio/method.UnixFDList.get.html)
> index_ specifies the index of the file descriptor to get. It is a programmer
> error for index_ to be out of range. Either use
> [g_unix_fd_list_lookup()](https://docs.gtk.org/gio/method.UnixFDList.lookup.html)
> to do a checked lookup, or check the index against the list length using
> [g_unix_fd_list_get_length()](https://docs.gtk.org/gio/method.UnixFDList.get_length.html).
I added bounds check as well.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/33059#discussion_r4132262585
PR Review Comment: https://git.openjdk.org/jdk/pull/33059#discussion_r4132200393
