On Fri, 22 Aug 2025 00:23:03 GMT, Alexander Zvegintsev <azveg...@openjdk.org> 
wrote:

>> src/java.desktop/unix/native/libawt_xawt/awt/gtk3_interface.c line 618:
>> 
>>> 616:         fp_g_string_free = dl_symbol("g_string_free");
>>> 617: 
>>> 618:         if (fp_glib_check_version != NULL) {
>> 
>> I don't know why we don't treat failing to find this symbol as a fatal error 
>> like we do for others such as this first one. dl_symbol will do a longjmp 
>>         fp_gtk_check_version = dl_symbol("gtk_check_version");
>>      
>> but for this one we just clear the error.
>>         /* GLib */
>>         fp_glib_check_version = dlsym(gtk3_libhandle, "glib_check_version");
>>         if (!fp_glib_check_version) {
>>             dlerror();
>>         }
>> 
>> @azvegint any idea ?
>
>> I don't know why we don't treat failing to find this symbol as a fatal error 
>> like we do for others such as this first one.
> 
> It looks like the `glib_check_version` was added but never used. The first 
> instance of its use appears to be the added check for the methods used for 
> Screencast. And it was overlooked that the `fp_glib_check_version` can be 
> `null`.
> 
> The `glib_check_version` is [available since 
> 2.6](https://docs.gtk.org/glib/func.check_version.html), and the 2.6.0 [was 
> released in 
> 2004](https://mail.gnome.org/archives/gnome-announce-list/2004-December/msg00036.html),
>  so I guess we can safely replace the `dlsym` with the `dl_symbol`.
> This will be better than remembering to protect it with a null check if we 
> decide to use `glib_check_version` later somewhere else.
> 
> 
> 
> -        fp_glib_check_version = dlsym(gtk3_libhandle, "glib_check_version");
> -        if (!fp_glib_check_version) {
> -            dlerror();
> -        }
> +        fp_glib_check_version = dl_symbol("glib_check_version");

So that would be the proper fix here, not the proposed one.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/26799#discussion_r2292666097

Reply via email to