[v3,1/1] package/libxml2: fix compilation with uclibc

3763 diff mbox series

The patch fixes the following errors and warnings raised by the
compilation of the library with uClibc:

encoding.c: In function ‘xmlEncInputChunk’:
encoding.c:2209:32: warning: comparison between pointer and integer
 2209 |     else if (handler->iconv_in != NULL) {
      |                                ^~
encoding.c: In function ‘xmlEncOutputChunk’:
encoding.c:2269:33: warning: comparison between pointer and integer
 2269 |     else if (handler->iconv_out != NULL) {
      |                                 ^~
encoding.c: In function ‘xmlCharEncCloseFunc’:
encoding.c:2681:29: warning: comparison between pointer and integer
 2681 |     if ((handler->iconv_out != NULL) || (handler->iconv_in != NULL)) {
      |                             ^~
encoding.c:2681:60: warning: comparison between pointer and integer
 2681 |     if ((handler->iconv_out != NULL) || (handler->iconv_in != NULL)) {
      |                                                            ^~
encoding.c:2683:32: warning: comparison between pointer and integer
 2683 |         if (handler->iconv_out != NULL) {
      |                                ^~
encoding.c:2686:32: error: assignment to ‘iconv_t’ {aka ‘long int’} from ‘void *’ makes integer from pointer without a cast [-Wint-conversion]
 2686 |             handler->iconv_out = NULL;
      |                                ^
encoding.c:2688:31: warning: comparison between pointer and integer
 2688 |         if (handler->iconv_in != NULL) {
      |                               ^~
encoding.c:2691:31: error: assignment to ‘iconv_t’ {aka ‘long int’} from ‘void *’ makes integer from pointer without a cast [-Wint-conversion]
 2691 |             handler->iconv_in = NULL;
      |                               ^
make[4]: *** [Makefile:1147: libxml2_la-encoding.lo] Error 1

The regression was partially introduced in version 2.1.0:
496a1cf59284 ("496a1cf59284 revamped the encoding support, added iconv support, so now libxml if")
and partially in version 2.2.3:
87b953957305 ("Large sync between my W3C base and Gnome's one:")

So the regression was already present in the first version of libxml2
(2.6.29) in Buildroot.

Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
---

Changes v2 -> v3:
- Update the commit message adding info about the version introduced the
  regression.

Changes v1 -> v2:
- Update the patch to the merged

 .../0001-Fix-compilation-with-uclibc.patch    | 114 ++++++++++++++++++
 1 file changed, 114 insertions(+)
 create mode 100644 package/libxml2/0001-Fix-compilation-with-uclibc.patch

Hello Dario,

On Fri, 27 Dec 2024 18:42:46 +0100
Dario Binacchi <dario.binacchi@amarulasolutions.com> wrote:

> The patch fixes the following errors and warnings raised by the
> compilation of the library with uClibc:
> 
> encoding.c: In function ‘xmlEncInputChunk’:
> encoding.c:2209:32: warning: comparison between pointer and integer
>  2209 |     else if (handler->iconv_in != NULL) {
>       |                                ^~
> encoding.c: In function ‘xmlEncOutputChunk’:
> encoding.c:2269:33: warning: comparison between pointer and integer
>  2269 |     else if (handler->iconv_out != NULL) {
>       |                                 ^~
> encoding.c: In function ‘xmlCharEncCloseFunc’:
> encoding.c:2681:29: warning: comparison between pointer and integer
>  2681 |     if ((handler->iconv_out != NULL) || (handler->iconv_in != NULL)) {
>       |                             ^~
> encoding.c:2681:60: warning: comparison between pointer and integer
>  2681 |     if ((handler->iconv_out != NULL) || (handler->iconv_in != NULL)) {
>       |                                                            ^~
> encoding.c:2683:32: warning: comparison between pointer and integer
>  2683 |         if (handler->iconv_out != NULL) {
>       |                                ^~
> encoding.c:2686:32: error: assignment to ‘iconv_t’ {aka ‘long int’} from ‘void *’ makes integer from pointer without a cast [-Wint-conversion]
>  2686 |             handler->iconv_out = NULL;
>       |                                ^
> encoding.c:2688:31: warning: comparison between pointer and integer
>  2688 |         if (handler->iconv_in != NULL) {
>       |                               ^~
> encoding.c:2691:31: error: assignment to ‘iconv_t’ {aka ‘long int’} from ‘void *’ makes integer from pointer without a cast [-Wint-conversion]
>  2691 |             handler->iconv_in = NULL;
>       |                               ^
> make[4]: *** [Makefile:1147: libxml2_la-encoding.lo] Error 1
> 
> The regression was partially introduced in version 2.1.0:
> 496a1cf59284 ("496a1cf59284 revamped the encoding support, added iconv support, so now libxml if")
> and partially in version 2.2.3:
> 87b953957305 ("Large sync between my W3C base and Gnome's one:")
> 
> So the regression was already present in the first version of libxml2
> (2.6.29) in Buildroot.
> 
> Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
> ---

Thanks for the patch and investigation. However the explanation was not
really complete: it is clearly impossible that this build failure
existed with uClibc since libxml2 was introduced in Buildroot. Indeed,
uClibc used to be the only C library in Buildroot, it has been the
default for many years, etc.

So in fact, the issue started occurring only since GCC 14.x was
introduced. It used to be a warning, and GCC 14.x turned it into a hard
build error. Also, it appears only with uClibc because uClibc defines
iconv_t as "long", while both glibc and musl define it as "void *".

I have updated the commit log with those details, I've added a
reference to the autobuilder failure being fixed in the commit log, and
updated the Upstream: link in the patch to point to the upstream commit.

Thanks for this great investigation work!

Thomas

On Fri, Dec 27, 2024 at 7:52 PM Dario Binacchi
<dario.binacchi@amarulasolutions.com> wrote:
>
> The patch fixes the following errors and warnings raised by the
> compilation of the library with uClibc:
>
> encoding.c: In function ‘xmlEncInputChunk’:
> encoding.c:2209:32: warning: comparison between pointer and integer
>  2209 |     else if (handler->iconv_in != NULL) {
>       |                                ^~
> encoding.c: In function ‘xmlEncOutputChunk’:
> encoding.c:2269:33: warning: comparison between pointer and integer
>  2269 |     else if (handler->iconv_out != NULL) {
>       |                                 ^~
> encoding.c: In function ‘xmlCharEncCloseFunc’:
> encoding.c:2681:29: warning: comparison between pointer and integer
>  2681 |     if ((handler->iconv_out != NULL) || (handler->iconv_in != NULL)) {
>       |                             ^~
> encoding.c:2681:60: warning: comparison between pointer and integer
>  2681 |     if ((handler->iconv_out != NULL) || (handler->iconv_in != NULL)) {
>       |                                                            ^~
> encoding.c:2683:32: warning: comparison between pointer and integer
>  2683 |         if (handler->iconv_out != NULL) {
>       |                                ^~
> encoding.c:2686:32: error: assignment to ‘iconv_t’ {aka ‘long int’} from ‘void *’ makes integer from pointer without a cast [-Wint-conversion]
>  2686 |             handler->iconv_out = NULL;
>       |                                ^
> encoding.c:2688:31: warning: comparison between pointer and integer
>  2688 |         if (handler->iconv_in != NULL) {
>       |                               ^~
> encoding.c:2691:31: error: assignment to ‘iconv_t’ {aka ‘long int’} from ‘void *’ makes integer from pointer without a cast [-Wint-conversion]
>  2691 |             handler->iconv_in = NULL;
>       |                               ^
> make[4]: *** [Makefile:1147: libxml2_la-encoding.lo] Error 1
>
> The regression was partially introduced in version 2.1.0:
> 496a1cf59284 ("496a1cf59284 revamped the encoding support, added iconv support, so now libxml if")
> and partially in version 2.2.3:
> 87b953957305 ("Large sync between my W3C base and Gnome's one:")
>
> So the regression was already present in the first version of libxml2
> (2.6.29) in Buildroot.

This patch appears to have introduced a regression for glibc based systems:

==4792== at 0x4FE7661: __gconv_close (gconv_close.c:33)
==4792== by 0x4FE70EE: iconv_close (iconv_close.c:34)
==4792== by 0x5AED422: xmlCharEncCloseFunc (encoding.c:2678)
==4792== by 0x587A78C: __pyx_f_4lxml_5etree__find_PyUCS4EncodingName
(etree.c:126676)
==4792== by 0x5888666: __pyx_pymod_exec_etree (etree.c:289773)
==4792== by 0x4A06345: PyModule_ExecDef (moduleobject.c:440)
==4792== by 0x4AE23A8: _imp_exec_dynamic_impl (import.c:3801)
==4792== by 0x4AE23A8: _imp_exec_dynamic (import.c.h:534)
==4792== by 0x4A046A3: cfunction_vectorcall_O (methodobject.c:509)
==4792== by 0x4AAA203: _PyEval_EvalFrameDefault (bytecodes.c:3263)
==4792== by 0x49B9D2B: _PyObject_VectorcallTstate (pycore_call.h:92)
==4792== by 0x49B9D2B: object_vacall (call.c:850)
==4792== by 0x49B9F44: PyObject_CallMethodObjArgs (call.c:911)
==4792== by 0x4AE648A: import_find_and_load (import.c:2781)
==4792== by 0x4AE648A: PyImport_ImportModuleLevelObject (import.c:2864)
==4792== Address 0x8 is not stack'd, malloc'd or (recently) free'd
==4792== Invalid read of size 8

>
> Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
> ---
>
> Changes v2 -> v3:
> - Update the commit message adding info about the version introduced the
>   regression.
>
> Changes v1 -> v2:
> - Update the patch to the merged
>
>  .../0001-Fix-compilation-with-uclibc.patch    | 114 ++++++++++++++++++
>  1 file changed, 114 insertions(+)
>  create mode 100644 package/libxml2/0001-Fix-compilation-with-uclibc.patch
>
> diff --git a/package/libxml2/0001-Fix-compilation-with-uclibc.patch b/package/libxml2/0001-Fix-compilation-with-uclibc.patch
> new file mode 100644
> index 000000000000..857f6ca2a2a1
> --- /dev/null
> +++ b/package/libxml2/0001-Fix-compilation-with-uclibc.patch
> @@ -0,0 +1,114 @@
> +From fc72e0833a4e5724aef604e2fd9adb1014cb4844 Mon Sep 17 00:00:00 2001
> +From: Dario Binacchi <dario.binacchi@amarulasolutions.com>
> +Date: Mon, 16 Dec 2024 17:23:23 +0100
> +Subject: [PATCH] Fix compilation with uclibc
> +MIME-Version: 1.0
> +Content-Type: text/plain; charset=UTF-8
> +Content-Transfer-Encoding: 8bit
> +
> +The patch fixes the following errors and warnings raised by the
> +compilation of the library with uClibc:
> +
> +encoding.c: In function ‘xmlEncInputChunk’:
> +encoding.c:2209:32: warning: comparison between pointer and integer
> + 2209 |     else if (handler->iconv_in != NULL) {
> +      |                                ^~
> +encoding.c: In function ‘xmlEncOutputChunk’:
> +encoding.c:2269:33: warning: comparison between pointer and integer
> + 2269 |     else if (handler->iconv_out != NULL) {
> +      |                                 ^~
> +encoding.c: In function ‘xmlCharEncCloseFunc’:
> +encoding.c:2681:29: warning: comparison between pointer and integer
> + 2681 |     if ((handler->iconv_out != NULL) || (handler->iconv_in != NULL)) {
> +      |                             ^~
> +encoding.c:2681:60: warning: comparison between pointer and integer
> + 2681 |     if ((handler->iconv_out != NULL) || (handler->iconv_in != NULL)) {
> +      |                                                            ^~
> +encoding.c:2683:32: warning: comparison between pointer and integer
> + 2683 |         if (handler->iconv_out != NULL) {
> +      |                                ^~
> +encoding.c:2686:32: error: assignment to ‘iconv_t’ {aka ‘long int’} from ‘void *’ makes integer from pointer without a cast [-Wint-conversion]
> + 2686 |             handler->iconv_out = NULL;
> +      |                                ^
> +encoding.c:2688:31: warning: comparison between pointer and integer
> + 2688 |         if (handler->iconv_in != NULL) {
> +      |                               ^~
> +encoding.c:2691:31: error: assignment to ‘iconv_t’ {aka ‘long int’} from ‘void *’ makes integer from pointer without a cast [-Wint-conversion]
> + 2691 |             handler->iconv_in = NULL;
> +      |                               ^
> +make[4]: *** [Makefile:1147: libxml2_la-encoding.lo] Error 1
> +
> +Signed-off-by: Dario Binacchi <dario.binacchi@amarulasolutions.com>
> +Upstream: https://gitlab.gnome.org/GNOME/libxml2/-/merge_requests/292
> +---
> + encoding.c | 20 ++++++++++----------
> + 1 file changed, 10 insertions(+), 10 deletions(-)
> +
> +diff --git a/encoding.c b/encoding.c
> +index 14ffafddbc02..41ecde1885e4 100644
> +--- a/encoding.c
> ++++ b/encoding.c
> +@@ -1264,7 +1264,7 @@ DECLARE_ISO_FUNCS(16)
> + #endif /* LIBXML_ISO8859X_ENABLED */
> +
> + #ifdef LIBXML_ICONV_ENABLED
> +-  #define EMPTY_ICONV , (iconv_t) 0, (iconv_t) 0
> ++  #define EMPTY_ICONV , (iconv_t) -1, (iconv_t) -1
> + #else
> +   #define EMPTY_ICONV
> + #endif
> +@@ -1389,8 +1389,8 @@ xmlNewCharEncodingHandler(const char *name,
> +     handler->name = up;
> +
> + #ifdef LIBXML_ICONV_ENABLED
> +-    handler->iconv_in = NULL;
> +-    handler->iconv_out = NULL;
> ++    handler->iconv_in = (iconv_t) -1;
> ++    handler->iconv_out = (iconv_t) -1;
> + #endif
> + #ifdef LIBXML_ICU_ENABLED
> +     handler->uconv_in = NULL;
> +@@ -2200,7 +2200,7 @@ xmlEncInputChunk(xmlCharEncodingHandler *handler, unsigned char *out,
> +         }
> +     }
> + #ifdef LIBXML_ICONV_ENABLED
> +-    else if (handler->iconv_in != NULL) {
> ++    else if (handler->iconv_in != (iconv_t) -1) {
> +         ret = xmlIconvWrapper(handler->iconv_in, out, outlen, in, inlen);
> +     }
> + #endif /* LIBXML_ICONV_ENABLED */
> +@@ -2260,7 +2260,7 @@ xmlEncOutputChunk(xmlCharEncodingHandler *handler, unsigned char *out,
> +         }
> +     }
> + #ifdef LIBXML_ICONV_ENABLED
> +-    else if (handler->iconv_out != NULL) {
> ++    else if (handler->iconv_out != (iconv_t) -1) {
> +         ret = xmlIconvWrapper(handler->iconv_out, out, outlen, in, inlen);
> +     }
> + #endif /* LIBXML_ICONV_ENABLED */
> +@@ -2672,17 +2672,17 @@ xmlCharEncCloseFunc(xmlCharEncodingHandler *handler) {
> +      * Iconv handlers can be used only once, free the whole block.
> +      * and the associated icon resources.
> +      */
> +-    if ((handler->iconv_out != NULL) || (handler->iconv_in != NULL)) {
> ++    if ((handler->iconv_out != (iconv_t) -1) || (handler->iconv_in != (iconv_t) -1)) {
> +         tofree = 1;
> +-      if (handler->iconv_out != NULL) {
> ++      if (handler->iconv_out != (iconv_t) -1) {
> +           if (iconv_close(handler->iconv_out))
> +               ret = -1;
> +-          handler->iconv_out = NULL;
> ++          handler->iconv_out = (iconv_t) -1;
> +       }
> +-      if (handler->iconv_in != NULL) {
> ++      if (handler->iconv_in != (iconv_t) -1) {
> +           if (iconv_close(handler->iconv_in))
> +               ret = -1;
> +-          handler->iconv_in = NULL;
> ++          handler->iconv_in = (iconv_t) -1;
> +       }
> +     }
> + #endif /* LIBXML_ICONV_ENABLED */
> +--
> +2.43.0
> +
> --
> 2.43.0
>
> _______________________________________________
> buildroot mailing list
> buildroot@buildroot.org
> https://lists.buildroot.org/mailman/listinfo/buildroot

To unsubscribe from this group and stop receiving emails from it, send an email to linux-amarula+unsubscribe@amarulasolutions.com.