Re: [PATCH] string, wchar: avoid some namespace pollution

[Bruno Haible]

Paul Eggert did on 2021-09-07:
> diff --git a/lib/string.in.h b/lib/string.in.h
> index fa2e40c25..6214b5578 100644
> --- a/lib/string.in.h
> +++ b/lib/string.in.h
> @@ -47,9 +47,6 @@
>  /* NetBSD 5.0 mis-defines NULL.  */
>  #include <stddef.h>
>  
> -/* Get free().  */
> -#include <stdlib.h>
> -
>  /* MirBSD defines mbslen as a macro.  */
>  #if @GNULIB_MBSLEN@ && defined __MirBSD__
>  # include <wchar.h>
> @@ -86,6 +83,12 @@
>  
>  /* The definition of _GL_WARN_ON_USE is copied here.  */
>  
> +/* Declare 'free' if needed for _GL_ATTRIBUTE_DEALLOC_FREE.  */
> +#if (@REPLACE_FREE@ && !defined free \
> +     && !(defined __cplusplus && defined GNULIB_NAMESPACE))
> +# define free rpl_free
> +#endif
> +_GL_EXTERN_C void free (void *);
>  
>  /* Clear a block of memory.  The compiler will not delete a call to
>     this function, even if the block is dead after the call.  */

Simon Josefsson reported in
<https://lists.gnu.org/archive/html/bug-gnulib/2021-09/msg00042.html>:

> since a few days libidn fails to
> build with a reference to rpl_free, that looks related to this thread
> and the recent gnulib changes for the free module:
>
> https://oss-fuzz-build-logs.storage.googleapis.com/log-002189ee-3c98-45b8-9bde-3bfb20317e9a.txt

Thanks for the heads-up, Simon. I debugged it, and indeed the cause is
in gnulib, not in libidn. The patch below fixes it.

The situation is as follows:

libidn makes two uses of gnulib-tool:
  - one for subdirectory lib/gl/, with module 'free-posix' included
    (indirectly),
  - one for subdirectory gl/, with module 'free-posix' NOT included.

Accordingly, the stdlib.h files are correctly generated like this:

lib/gl/stdlib.h:
#if 1       /* because here, GNULIB_FREE_POSIX evaluates to 1 */
# if 1
#  if !(defined __cplusplus && defined GNULIB_NAMESPACE)
#   undef free
#   define free rpl_free
#  endif

gl/stdlib.h:
#if 0       /* because here, GNULIB_FREE_POSIX evaluates to 0 */
# if 1
#  if !(defined __cplusplus && defined GNULIB_NAMESPACE)
#   undef free
#   define free rpl_free
#  endif

In the compiled library, rpl_free is defined for use inside the library
but not from outside. (This is also correct: the library is namespace-
clean.)
$ nm ../lib/.libs/libidn.so | grep free
                 U free@@GLIBC_2.2.5
0000000000006ab0 T idn_free
0000000000008380 t rpl_free

The problem is that the generated gl/string.h does a '#define free rpl_free',
although it shouldn't. The "gcc ... -E -dD" output of the compilation unit
(outside the library) has

  # 50 "../gl/string.h" 2 3
  # 596 "../gl/string.h" 3
  extern void free (void *);
  #define free rpl_free

The newly added code snippet in string.h should not only test @REPLACE_FREE@
but also @GNULIB_FREE_POSIX@.

In general, the rule to remember is:

  =====================================================================
  Variables like REPLACE_XYZ can be tested
    - in m4 code,
    - in configure.ac snippets (that are part of a module),
    - and in *.in.h files but *only* when guarded by a test of the
      corresponding GNULIB_XYZ symbol.
  =====================================================================


2021-09-18  Bruno Haible  <bruno@clisp.org>

        string, wchar: Don't cause link errors for rpl_free (regr. 2021-09-07).
        * lib/string.in.h (free, rpl_free): Consider GNULIB_FREE_POSIX variable.
        * lib/wchar.in.h (free, rpl_free): Likewise.
        * m4/string_h.m4 (gl_STRING_H_REQUIRE_DEFAULTS): Require module
        indicator variable initializations from the stdlib module.
        * m4/wchar_h.m4 (gl_WCHAR_H_REQUIRE_DEFAULTS): Likewise.
        * modules/string (Makefile.am): Substitute GNULIB_FREE_POSIX in 
string.h.
        * modules/wchar (Makefile.am): Substitute GNULIB_FREE_POSIX in wchar.h.

diff --git a/lib/string.in.h b/lib/string.in.h
index 8977153c8..8d77ae380 100644
--- a/lib/string.in.h
+++ b/lib/string.in.h
@@ -84,12 +84,14 @@
 /* The definition of _GL_WARN_ON_USE is copied here.  */
 
 /* Declare 'free' if needed for _GL_ATTRIBUTE_DEALLOC_FREE.  */
-#if (@REPLACE_FREE@ && !defined free \
-     && !(defined __cplusplus && defined GNULIB_NAMESPACE))
 _GL_EXTERN_C void free (void *);
-# define free rpl_free
-#endif
+#if @GNULIB_FREE_POSIX@
+# if (@REPLACE_FREE@ && !defined free \
+      && !(defined __cplusplus && defined GNULIB_NAMESPACE))
+#  define free rpl_free
 _GL_EXTERN_C void free (void *);
+# endif
+#endif
 
 /* Clear a block of memory.  The compiler will not delete a call to
    this function, even if the block is dead after the call.  */
diff --git a/lib/wchar.in.h b/lib/wchar.in.h
index acb9d4ea6..f13379ad8 100644
--- a/lib/wchar.in.h
+++ b/lib/wchar.in.h
@@ -147,12 +147,14 @@ typedef int rpl_mbstate_t;
 #endif
 
 /* Declare 'free' if needed for _GL_ATTRIBUTE_DEALLOC_FREE.  */
-#if (@REPLACE_FREE@ && !defined free \
-     && !(defined __cplusplus && defined GNULIB_NAMESPACE))
 _GL_EXTERN_C void free (void *);
-# define free rpl_free
-#endif
+#if @GNULIB_FREE_POSIX@
+# if (@REPLACE_FREE@ && !defined free \
+      && !(defined __cplusplus && defined GNULIB_NAMESPACE))
+#  define free rpl_free
 _GL_EXTERN_C void free (void *);
+# endif
+#endif
 
 /* Convert a single-byte character to a wide character.  */
 #if @GNULIB_BTOWC@
diff --git a/m4/string_h.m4 b/m4/string_h.m4
index 80d1e5875..9871dac8b 100644
--- a/m4/string_h.m4
+++ b/m4/string_h.m4
@@ -5,7 +5,7 @@
 # gives unlimited permission to copy and/or distribute it,
 # with or without modifications, as long as this notice is preserved.
 
-# serial 32
+# serial 33
 
 # Written by Paul Eggert.
 
@@ -93,6 +93,8 @@ AC_DEFUN([gl_STRING_H_REQUIRE_DEFAULTS],
     gl_MODULE_INDICATOR_INIT_VARIABLE([GNULIB_MDA_STRDUP], [1])
   ])
   m4_require(GL_MODULE_INDICATOR_PREFIX[_STRING_H_MODULE_INDICATOR_DEFAULTS])
+  dnl Make sure the shell variable for GNULIB_FREE_POSIX is initialized.
+  m4_require(GL_MODULE_INDICATOR_PREFIX[_STDLIB_H_MODULE_INDICATOR_DEFAULTS])
   AC_REQUIRE([gl_STRING_H_DEFAULTS])
 ])
 
diff --git a/m4/wchar_h.m4 b/m4/wchar_h.m4
index 818b3192e..d69dbe67d 100644
--- a/m4/wchar_h.m4
+++ b/m4/wchar_h.m4
@@ -7,7 +7,7 @@ dnl with or without modifications, as long as this notice is 
preserved.
 
 dnl Written by Eric Blake.
 
-# wchar_h.m4 serial 53
+# wchar_h.m4 serial 54
 
 AC_DEFUN_ONCE([gl_WCHAR_H],
 [
@@ -189,6 +189,8 @@ AC_DEFUN([gl_WCHAR_H_REQUIRE_DEFAULTS],
     gl_MODULE_INDICATOR_INIT_VARIABLE([GNULIB_MDA_WCSDUP], [1])
   ])
   m4_require(GL_MODULE_INDICATOR_PREFIX[_WCHAR_H_MODULE_INDICATOR_DEFAULTS])
+  dnl Make sure the shell variable for GNULIB_FREE_POSIX is initialized.
+  m4_require(GL_MODULE_INDICATOR_PREFIX[_STDLIB_H_MODULE_INDICATOR_DEFAULTS])
   AC_REQUIRE([gl_WCHAR_H_DEFAULTS])
 ])
 
diff --git a/modules/string b/modules/string
index 306834591..e9c315399 100644
--- a/modules/string
+++ b/modules/string
@@ -75,6 +75,7 @@ string.h: string.in.h $(top_builddir)/config.status 
$(CXXDEFS_H) $(ARG_NONNULL_H
              -e 's/@''GNULIB_STRVERSCMP''@/$(GNULIB_STRVERSCMP)/g' \
              -e 's/@''GNULIB_MDA_MEMCCPY''@/$(GNULIB_MDA_MEMCCPY)/g' \
              -e 's/@''GNULIB_MDA_STRDUP''@/$(GNULIB_MDA_STRDUP)/g' \
+             -e 's/@''GNULIB_FREE_POSIX''@/$(GNULIB_FREE_POSIX)/g' \
              < $(srcdir)/string.in.h | \
          sed -e 's|@''HAVE_EXPLICIT_BZERO''@|$(HAVE_EXPLICIT_BZERO)|g' \
              -e 's|@''HAVE_FFSL''@|$(HAVE_FFSL)|g' \
diff --git a/modules/wchar b/modules/wchar
index d34cb6a22..becd7968a 100644
--- a/modules/wchar
+++ b/modules/wchar
@@ -79,6 +79,7 @@ wchar.h: wchar.in.h $(top_builddir)/config.status 
$(CXXDEFS_H) $(ARG_NONNULL_H)
              -e 's/@''GNULIB_WCSWIDTH''@/$(GNULIB_WCSWIDTH)/g' \
              -e 's/@''GNULIB_WCSFTIME''@/$(GNULIB_WCSFTIME)/g' \
              -e 's/@''GNULIB_MDA_WCSDUP''@/$(GNULIB_MDA_WCSDUP)/g' \
+             -e 's/@''GNULIB_FREE_POSIX''@/$(GNULIB_FREE_POSIX)/g' \
              < $(srcdir)/wchar.in.h | \
          sed -e 's|@''HAVE_WINT_T''@|$(HAVE_WINT_T)|g' \
              -e 's|@''HAVE_BTOWC''@|$(HAVE_BTOWC)|g' \