RFR: 8352162: Update libxml2 to 2.13.8 [v6]

Mon Apr 21 20:36:59 UTC 2025

On Mon, 21 Apr 2025 17:28:12 GMT, Jay Bhaskar <jbhaskar at openjdk.org> wrote:

>> The upgrade is required to address several known issues from the previous version to improve stability and performance.
>
> Jay Bhaskar has updated the pull request incrementally with one additional commit since the last revision:
> 
>   remove unused files

I left some inline feedback on the libxml2 update.

I provided one more example of a libxml2 file that was not new in the 2.13.x release, but which was added as part of this PR (there are 30 such files. I will list them in a separate comment).

modules/javafx.web/src/main/native/Source/ThirdParty/libxml/linux/config.h line 29:

> 27: 
> 28: /* getentropy */
> 29: #define HAVE_GETENTROPY 0

The pattern in `config.h` is to comment out the define for an unused feature, not to define it to 0. This is important because most of the conditional defines are checked using `#ifdef` or `#if defined`, and those macros don't care what the value is.

This needs to be fixed. If this feature is now needed, the define it as follows:

/* Define to 1 if you have the 'getentropy' function. */
#define HAVE_GETENTROPY 1

else comment it out as follows:

/* Define to 1 if you have the 'getentropy' function. */
/* #undef HAVE_GETENTROPY */

modules/javafx.web/src/main/native/Source/ThirdParty/libxml/linux/config.h line 67:

> 65: 
> 66: /* Define to 1 if you have the <poll.h> header file. */
> 67: /* #undef HAVE_POLL_H */

Is this no longer needed?

modules/javafx.web/src/main/native/Source/ThirdParty/libxml/linux/config.h line 97:

> 95: 
> 96: /* Define to 1 if you have the <sys/random.h> header file. */
> 97: #define HAVE_SYS_RANDOM_H 0

This also needs to be fixed (either define it to 1 or else comment it out).

modules/javafx.web/src/main/native/Source/ThirdParty/libxml/linux/include/libxml/xmlversion.h line 322:

> 320:  * the string suffix used by dynamic modules (usually shared libraries)
> 321:  */
> 322: #define LIBXML_MODULE_EXTENSION ".so

This looks wrong. It would almost certainly fail to compile if the code path were active. You might want to fix it, although it is unused.

modules/javafx.web/src/main/native/Source/ThirdParty/libxml/mac/config.h line 29:

> 27: 
> 28: /* getentropy */
> 29: #define HAVE_GETENTROPY 1

I think the comment should be:

/* Define to 1 if you have the 'getentropy' function. */

Can you check this?

modules/javafx.web/src/main/native/Source/ThirdParty/libxml/mac/include/xmlversion.h line 1:

> 1: /*

This file is not used and should be deleted. It looks like a spurious copy of `mac/include/libxml/xmlversion.h`.

modules/javafx.web/src/main/native/Source/ThirdParty/libxml/src/CMakeLists.txt line 1:

> 1: cmake_minimum_required(VERSION 3.18)

I don't think we use this file in our build. We should remove it if not.

modules/javafx.web/src/main/native/Source/ThirdParty/libxml/src/SAX.c line 2:

> 1: /*
> 2:  * SAX.c : Old SAX v1 handlers to build a tree.

Here is an example of what I mentioned earlier: This is a file that we didn't used to take from the upstream. I don't think we use it or need it.

modules/javafx.web/src/main/native/Source/ThirdParty/libxml/src/dict.c line 933:

> 931: #else
> 932:   // This block will compile on macOS (and any non-Linux system) if HAVE_GETENTROPY is defined
> 933:   #if defined(HAVE_GETENTROPY) && !defined(__linux__)

Can you check whether this is needed if `HAVE_GETENTROPY` is left undefined? I suspect it will no longer be necessary, and it would be better not to have local mods to upstream files.

If a modification _is_ needed, then we will need a clear comment with the changes, noting that this is a JavaFX-specific addition.

modules/javafx.web/src/main/native/Source/ThirdParty/libxml/src/tree.c line 5862:

> 5860:  *
> 5861:  * Merge the second text node into the first. The second node is
> 5862:  * unlinked and freed.

It looks like you missed applying one of the changes from 2.13.7. Replace the above two lines with:

 * Merge the second text node into the first. If @first is NULL,
 * @second is returned. Otherwise, the second node is unlinked and
 * freed.

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

PR Review: https://git.openjdk.org/jfx/pull/1785#pullrequestreview-2781990520
PR Review Comment: https://git.openjdk.org/jfx/pull/1785#discussion_r2052845497
PR Review Comment: https://git.openjdk.org/jfx/pull/1785#discussion_r2052864462
PR Review Comment: https://git.openjdk.org/jfx/pull/1785#discussion_r2052861880
PR Review Comment: https://git.openjdk.org/jfx/pull/1785#discussion_r2052866676
PR Review Comment: https://git.openjdk.org/jfx/pull/1785#discussion_r2052869329
PR Review Comment: https://git.openjdk.org/jfx/pull/1785#discussion_r2052877696
PR Review Comment: https://git.openjdk.org/jfx/pull/1785#discussion_r2052878445
PR Review Comment: https://git.openjdk.org/jfx/pull/1785#discussion_r2052926586
PR Review Comment: https://git.openjdk.org/jfx/pull/1785#discussion_r2052935612
PR Review Comment: https://git.openjdk.org/jfx/pull/1785#discussion_r2052940458