On Fri, 22 Aug 2025 04:34:00 GMT, Phil Race <p...@openjdk.org> wrote:
>>> 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. > > 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. > > ```diff > - 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"); > ``` done ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/26799#discussion_r2300094456
