Re: [PATCH] free: preserve errno

[Paul Eggert]

Thanks for fixing that.

It is amusing that after 45 years we are still fussing with the semantics of 
'free'. You'd think it'd be simple, but no

Re: [PATCH] free: preserve errno

[Bruno Haible]

Pádraig Brady wrote:
> coreutils is now failing to build with:
> 
>lib/free.c:28:1: error: no previous declaration
>  for 'rpl_free' [-Werror=missing-declarations]
> 28 | rpl_free (void *p)
>| ^~~~
> 
> In early 2008 coreutils removed use of the then deprecated "free" module,
> but I now see 'canonicalize' explicitly depends on this now undeprecated 
> module.

What I see is a warning:

free.c:28:1: warning: no previous declaration for ‘rpl_free’ 
[-Wmissing-declarations]
 rpl_free (void *p)
 ^

Recall that Gnulib does not guarantee warning-free compilation on any
platform.

This patch should fix the warning, by using the modern idioms (that also support
C++ compilation). But there are more issues in this area; more patches to come.


2020-12-17  Bruno Haible  

free: Fix warning.
Reported by Pádraig Brady  in
.
* lib/stdlib.in.h (free): New declaration.
* m4/stdlib_h.m4 (gl_STDLIB_H): Test whether 'free' is declared.
(gl_STDLIB_H_DEFAULTS): Initialize GNULIB_FREE, REPLACE_FREE.
* modules/stdlib (Makefile.am): Substitute GNULIB_FREE, REPLACE_FREE.
* m4/free.m4 (gl_FUNC_FREE): Set REPLACE_FREE, instead of defining
'free' as a macro here.
* modules/free (Depends-on): Add stdlib.
(configure.ac): Test REPLACE_FREE. Invoke gl_STDLIB_MODULE_INDICATOR.

diff --git a/lib/stdlib.in.h b/lib/stdlib.in.h
index a76fbdc..a512459 100644
--- a/lib/stdlib.in.h
+++ b/lib/stdlib.in.h
@@ -284,6 +284,27 @@ _GL_CXXALIAS_SYS (fcvt, char *,
 _GL_CXXALIASWARN (fcvt);
 #endif
 
+#if @GNULIB_FREE@
+# if @REPLACE_FREE@
+#  if !(defined __cplusplus && defined GNULIB_NAMESPACE)
+#   undef free
+#   define free rpl_free
+#  endif
+_GL_FUNCDECL_RPL (free, void, (void *ptr));
+_GL_CXXALIAS_RPL (free, void, (void *ptr));
+# else
+_GL_CXXALIAS_SYS (free, void, (void *ptr));
+# endif
+# if __GLIBC__ >= 2
+_GL_CXXALIASWARN (free);
+# endif
+#elif defined GNULIB_POSIXCHECK
+# undef free
+/* Assume free is always declared.  */
+_GL_WARN_ON_USE (free, "free is not future POSIX compliant everywhere - "
+ "use gnulib module free for portability");
+#endif
+
 /* On native Windows, map 'gcvt' to '_gcvt', so that -loldnames is not
required.  In C++ with GNULIB_NAMESPACE, avoid differences between
platforms by defining GNULIB_NAMESPACE::gcvt on all platforms that have
diff --git a/m4/free.m4 b/m4/free.m4
index 62daa8e..9f108c1 100644
--- a/m4/free.m4
+++ b/m4/free.m4
@@ -1,5 +1,4 @@
-# Check whether free (NULL) is supposed to work.
-
+# free.m4 serial 1
 # Copyright (C) 2003-2005, 2009-2020 Free Software Foundation, Inc.
 # This file is free software; the Free Software Foundation
 # gives unlimited permission to copy and/or distribute it,
@@ -16,6 +15,7 @@
 
 AC_DEFUN([gl_FUNC_FREE],
 [
+  AC_REQUIRE([gl_STDLIB_H_DEFAULTS])
   AC_REQUIRE([AC_CANONICAL_HOST])
   AC_CACHE_CHECK([whether free (NULL) is known to work],
 [gl_cv_func_free],
@@ -67,10 +67,7 @@ AC_DEFUN([gl_FUNC_FREE],
 
   case $gl_cv_func_free,$gl_cv_func_free_preserves_errno in
*yes,*yes) ;;
-   *)
-AC_DEFINE([free], [rpl_free],
-  [Define to rpl_free if the replacement function should be used.])
-;;
+   *) REPLACE_FREE=1 ;;
   esac
 ])
 
diff --git a/m4/stdlib_h.m4 b/m4/stdlib_h.m4
index 8b3cfe3..6b519c4 100644
--- a/m4/stdlib_h.m4
+++ b/m4/stdlib_h.m4
@@ -1,4 +1,4 @@
-# stdlib_h.m4 serial 52
+# stdlib_h.m4 serial 53
 dnl Copyright (C) 2007-2020 Free Software Foundation, Inc.
 dnl This file is free software; the Free Software Foundation
 dnl gives unlimited permission to copy and/or distribute it,
@@ -22,7 +22,7 @@ AC_DEFUN([gl_STDLIB_H],
 #if HAVE_RANDOM_H
 # include 
 #endif
-]], [_Exit aligned_alloc atoll canonicalize_file_name
+]], [_Exit aligned_alloc atoll canonicalize_file_name free
 getloadavg getsubopt grantpt
 initstate initstate_r mbtowc mkdtemp mkostemp mkostemps mkstemp mkstemps
 posix_memalign posix_openpt ptsname ptsname_r qsort_r
@@ -62,6 +62,7 @@ AC_DEFUN([gl_STDLIB_H_DEFAULTS],
   GNULIB_ATOLL=0; AC_SUBST([GNULIB_ATOLL])
   GNULIB_CALLOC_POSIX=0;  AC_SUBST([GNULIB_CALLOC_POSIX])
   GNULIB_CANONICALIZE_FILE_NAME=0;  AC_SUBST([GNULIB_CANONICALIZE_FILE_NAME])
+  GNULIB_FREE=0;  AC_SUBST([GNULIB_FREE])
   GNULIB_GETLOADAVG=0;AC_SUBST([GNULIB_GETLOADAVG])
   GNULIB_GETSUBOPT=0; AC_SUBST([GNULIB_GETSUBOPT])
   GNULIB_GRANTPT=0;   AC_SUBST([GNULIB_GRANTPT])
@@ -140,6 +141,7 @@ AC_DEFUN([gl_STDLIB_H_DEFAULTS],
   REPLACE_ALIGNED_ALLOC=0;   AC_SUBST([REPLACE_ALIGNED_ALLOC])
   REPLACE_CALLOC=0;  AC_SUBST([REPLACE_CALLOC])
   REPLACE_CANONICALIZE_FILE_NAME=0;  AC_SUBST([REPLACE_CANONICALIZE_FILE_NAME])
+  REPLACE_FREE=0;AC_SUBST([REPLACE_FREE])
   REPLACE_INITSTATE=0;   AC_SUBST([REPLACE_INITSTATE])
   REPLACE_MALLOC=0;  AC_SUBST([REPLACE_MALLOC])
   REPLACE_MBTOWC=0;  AC_S

Re: Make more use of idx_t

[Bruno Haible]

Paul Eggert wrote:
> If we're 
> going to have a macro that stands for PTRDIFF_WIDTH - 1 it'd probably be 
> better to use a different naming convention, to avoid that confusion.

Makes sense. Yes, IDX_VALUE_BITS would be a better name than IDX_WIDTH.

Bruno

Re: [PATCH] free: preserve errno

[Pádraig Brady]

On 17/12/2020 09:54, Paul Eggert wrote:

* lib/free.c (rpl_free): Preserve errno.  Check for null only if
CANNOT_FREE_NULL is defined, as an optimization for POSIX 2008
platforms that do not preserve errno.
* m4/free.m4 (gl_FUNC_FREE): Check whether free preserves errno.
Also, define CANNOT_FREE_NULL if free cannot free NULL.
* modules/free (configure.ac): Also replace 'free' if
it does not preserve errno.


coreutils is now failing to build with:

  lib/free.c:28:1: error: no previous declaration
for 'rpl_free' [-Werror=missing-declarations]
   28 | rpl_free (void *p)
  | ^~~~

In early 2008 coreutils removed use of the then deprecated "free" module,
but I now see 'canonicalize' explicitly depends on this now undeprecated module.

I've worked around the issue,
but I'm not sure how you'd prefer to handle it in gnulib.

thanks,
Pádraig

Re: [PATCH 1/6] canonicalize-lgpl: fix EOVERFLOW bug

[Paul Eggert]

On 12/11/20 5:03 AM, Adhemerval Zanella wrote:


I have sent a similar fix to reviews for this very issue for glibc
(which is based on the canonicalize-lgpl) along with other fixes that
you might take a look at [1].


Thanks, I looked at that when composing the patches I just now installed 
into Gnulib, which also attempt to make future glibc merges easier. I think 
these patches address the issues mentioned in [1] (though sometimes in 
different ways), except they don't do what this patch does:


https://patchwork.sourceware.org/project/glibc/patch/20201027143531.2448132-4-adhemerval.zane...@linaro.org/

This patch seems incomplete, since it doesn't address the issue that 
canonicalize_file_name ("/a/b/.") incorrectly returns "/a/b" even when /a/b 
is a regular file.


Come to think of it, the current Gnulib is suboptimal in that it uses stat 
("/a/b/foo/", ...) to test for the existence of a directory /a/b/foo. It 
could use faccessat (AT_FDCWD, "/a/b/foo/", F_OK, AT_EACCESS), which is 
often cheaper as it needn't fill in the stat structure.


I'll try to make time to look into these two issues, which are somewhat 
related in the implementations of canonicalize-lgpl and canonicalize.

Re: [PATCH] tmpdir: Add function to create a unique temp directory.

[John Darrington]

On Thu, Dec 17, 2020 at 12:26:20AM +0100, Bruno Haible wrote:
 
 So, it differs from 'create_temp_dir' only in the aspect that it does NOT
 do automatic cleanup.
 
 Since you are already aware of the 'clean-temp' module — you contributed to
 it 8 years ago :) — what could we write in the documentation, what is the
 advantage of your proposed function?

I'd forgotten about that.   I guess there is little advantage except in the
implementation.  See below

 * Given that 'tmpdir', so far, is an auxiliary module (used by other 
modules),
   I don't like to add higher-level function to it. I would prefer if the
   higher-level function were in a separate module.
   We can rename the existing module 'tmpdir' to, say, 'find-tmpdir', if 
that
   helps.


I think that would make sense.  But so far as I can tell, path_search is used
only by the module clean-temp so why bother with it at all?  Why not move the
function into the clean-temp module and obsolete the tmpdir module?

 
 * The comment "TRY_TMPDIR is ignored if BASEDIR is non-null." suggests that
   it would be better to have two different functions, instead of trying to
   force it into one.
 
 * Much of the code looks like a copy of the 'path_search' function. Why not
   use 'path_search'? What is missing?
 
This was the original motivation for the change.  The path_search function in
its existing form is a pain to use:

One must pass it a buffer containing enough bytes to contain the result.  But
until you've called it, one doesn't know how many bytes are "enough".  One
solution is to dynamically allocate a buffer of 2  bytes, and call path_search
in a loop, increasing the buffer size each iteration, until it succeeds.  But
that of course is hardly efficient.

Another solution is to declare a buffer of length PATH_MAX but PATH_MAX does not
exist on the Hurd.   I see that what the clean-temp module does is to define
PATH_MAX as 1024 if it is not already defined.   This can be problematic for
two reasons:

1.  How do you know that 1024 is enough.

2.  What happens if the operating system *does* define PATH_MAX, but defines it
to something very large: eg:  0x  ?
In this case, clean-temp will fail when it gets to the line

xtemplate = (char *) xmalloca (PATH_MAX);


So I think what really needs to happen is, that path_search needs to be
rewritten (and renamed) so that it dynamically allocates its output, and
clean-temp needs to be updated to use the rewritten version.

J'

Re: [PATCH 2/6] canonicalize: fix EOVERFLOW bug

[Paul Eggert]

On 12/14/20 10:05 AM, Bruno Haible wrote:


But canonicalize-lgpl.c had similar changes, and it has a similar failure
on AIX 7.2, still today:

   ../../gltests/test-canonicalize-lgpl.c:100: assertion 'result == NULL' failed
   FAIL test-canonicalize-lgpl (exit status: 134)

Probably your changes to canonicalize.c could be applied to canonicalize-lgpl.c.


Thanks for mentioning that. While looking into it, I discovered several 
other correctness problems with canonicalize and canonicalize-lgpl in rare 
situations, and after looking at both of them for a while (along with glibc 
stdlib/canonicalize.c) decided to also lessen the differences between glibc 
and Gnulib to make a merge easier, and eventually came up with the attached 
patches, which I installed into Gnulib. The last patch fixes the AIX bug you 
mentioned.


These patches assume the 'free' patch I installed an hour or so ago, so that 
'free' preserves errno.


I plan to reply to Adhemerval shortly.
From 8306b5c9dd933e1c26db0a9eb35b94a863d7e5bc Mon Sep 17 00:00:00 2001
From: Paul Eggert 
Date: Mon, 14 Dec 2020 11:59:20 -0800
Subject: [PATCH 1/4] canonicalize-lgpl: simplify merge to glibc
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This patch lessens the differences between git glibc
stdlib/canonicalize.c and lib/canonicalize-lgpl.c.
The (perhaps wishful) goal is to make them identical.
* lib/canonicalize-lgpl.c [!_LIBC]:
Include , not config.h.
Omit an unnecessary (!HAVE_CANONICALIZE_FILE_NAME ||
!FUNC_REALPATH_WORKS || defined _LIBC) #if.
Do not include alloca.h, since we use malloca now.
[_LIBC]: Include , and define dummy macros
FILE_SYSTEM_PREFIX_LEN, IS_ABSOLUTE_FILE_NAME, ISSLASH, malloca,
freea so that the mainline code can be kept #ifdef free.
[!_LIBC]: Remove dummy macros for SHLIB_COMPAT, versioned_symbol,
compat_symbol, weak_alias, __set_errno since libc-config.h does that.
Add redirecting macros __mempcpy, __pathconf, __rawmemchr,
__eloop_threshold.  All uses of their definiens changed.
(SIZE_MAX): Remove; no longer needed.
(alloc_failed): Remove, and remove all instances.
No need for alloc_failed now that free preserves errno.
(__realpath): Default path_max to 1024 instead of 8192, as that’s
the glibc tradition and is safer when the 2nd argument is null.
Use __rawmemchr instead of strchr.
Use __mempcpy where appropriate.
Simplify test for overflow so that it does not need SIZE_MAX.
Do not preserve errno around free or freea calls; no longer needed.
Mark __realpath with libc_hidden_def.
* modules/canonicalize-lgpl (Depends-on): Add free, libc-config,
malloc-posix, mempcpy, realloc-posix, rawmemchr.
* modules/free: Now LGPLv2+, for canonicalize-lgpl.
---
 ChangeLog |  30 ++
 lib/canonicalize-lgpl.c   | 120 --
 modules/canonicalize-lgpl |   8 ++-
 modules/free  |   2 +-
 4 files changed, 74 insertions(+), 86 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index e97d82aa6..85e2b4c87 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,35 @@
 2020-12-17  Paul Eggert  
 
+	canonicalize-lgpl: simplify merge to glibc
+	This patch lessens the differences between git glibc
+	stdlib/canonicalize.c and lib/canonicalize-lgpl.c.
+	The (perhaps wishful) goal is to make them identical.
+	* lib/canonicalize-lgpl.c [!_LIBC]:
+	Include , not config.h.
+	Omit an unnecessary (!HAVE_CANONICALIZE_FILE_NAME ||
+	!FUNC_REALPATH_WORKS || defined _LIBC) #if.
+	Do not include alloca.h, since we use malloca now.
+	[_LIBC]: Include , and define dummy macros
+	FILE_SYSTEM_PREFIX_LEN, IS_ABSOLUTE_FILE_NAME, ISSLASH, malloca,
+	freea so that the mainline code can be kept #ifdef free.
+	[!_LIBC]: Remove dummy macros for SHLIB_COMPAT, versioned_symbol,
+	compat_symbol, weak_alias, __set_errno since libc-config.h does that.
+	Add redirecting macros __mempcpy, __pathconf, __rawmemchr,
+	__eloop_threshold.  All uses of their definiens changed.
+	(SIZE_MAX): Remove; no longer needed.
+	(alloc_failed): Remove, and remove all instances.
+	No need for alloc_failed now that free preserves errno.
+	(__realpath): Default path_max to 1024 instead of 8192, as that’s
+	the glibc tradition and is safer when the 2nd argument is null.
+	Use __rawmemchr instead of strchr.
+	Use __mempcpy where appropriate.
+	Simplify test for overflow so that it does not need SIZE_MAX.
+	Do not preserve errno around free or freea calls; no longer needed.
+	Mark __realpath with libc_hidden_def.
+	* modules/canonicalize-lgpl (Depends-on): Add free, libc-config,
+	malloc-posix, mempcpy, realloc-posix, rawmemchr.
+	* modules/free: Now LGPLv2+, for canonicalize-lgpl.
+
 	free: preserve errno
 	* lib/free.c (rpl_free): Preserve errno.  Check for null only if
 	CANNOT_FREE_NULL is defined, as an optimization for POSIX 2008
diff --git a/lib/canonicalize-lgpl.c b/lib/canonicalize-lgpl.c
index d090dcda1..693044ecc 100644
--- a/lib/canonicalize-lgpl.c
+++ b/lib/canonicalize-lgpl.c
@@ -2,18 +2,19 @

Re: Make more use of idx_t

[Paul Eggert]

On 12/6/20 3:32 AM, Bruno Haible wrote:

Paul Eggert wrote:

Those all look good to me.


OK, I pushed them.


When I looked into this a bit more when hacking on canonicalize-lgpl, I 
realized I spoke too soon. First, there's some cruft left over from when 
idx_t could be unsigned. More important, defining IDX_WIDTH to be 
PTRDIFF_WIDTH - 1 is confusing as this makes idx_t different from all other 
signed types in that IDX_WIDTH does not count the sign bit. I can see why it 
doesn't count the sign bit (the sign bit is always zero) but I can also see 
why it should (shift counts are limited by the *_WIDTH macros). If we're 
going to have a macro that stands for PTRDIFF_WIDTH - 1 it'd probably be 
better to use a different naming convention, to avoid that confusion. 
However, nobody's using any such macro now and possibly we'll never need it 
so in the meantime I merely substituted a comment for the #define in the 
attached patch.


Hope this is OK; further comments welcome of course.
From 03d1588fca2bffe92b08bb4f4ba69273f4843431 Mon Sep 17 00:00:00 2001
From: Paul Eggert 
Date: Wed, 16 Dec 2020 23:52:24 -0800
Subject: [PATCH] idx: simplify IDX_MAX, remove IDX_WIDTH
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* lib/idx.h (IDX_MAX): Simplify by removing obsolete reference
to UNSIGNED_IDX_T.
(IDX_WIDTH): Remove, since it’s not used and its value
arguably should be PTRDIFF_WIDTH anyway.
---
 ChangeLog |  8 
 lib/idx.h | 33 ++---
 2 files changed, 22 insertions(+), 19 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index d50539adc..445e2c074 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,11 @@
+2020-12-17  Paul Eggert  
+
+	idx: simplify IDX_MAX, remove IDX_WIDTH
+	* lib/idx.h (IDX_MAX): Simplify by removing obsolete reference
+	to UNSIGNED_IDX_T.
+	(IDX_WIDTH): Remove, since it’s not used and its value
+	arguably should be PTRDIFF_WIDTH anyway.
+
 2020-12-16  Bruno Haible  
 
 	posix_spawn_file_actions_addfchdir-tests: Rename test.
diff --git a/lib/idx.h b/lib/idx.h
index 0095467dc..ad7d31a2b 100644
--- a/lib/idx.h
+++ b/lib/idx.h
@@ -21,11 +21,12 @@
 /* Get ptrdiff_t.  */
 #include 
 
-/* Get PTRDIFF_WIDTH.  */
+/* Get PTRDIFF_MAX.  */
 #include 
 
 /* The type 'idx_t' holds an (array) index or an (object) size.
-   Its implementation is a signed integer type, capable of holding the values
+   Its implementation promotes to a signed integer type,
+   which can hold the values
  0..2^63-1 (on 64-bit platforms) or
  0..2^31-1 (on 32-bit platforms).
 
@@ -87,32 +88,26 @@
or to the C standard.  Several programming languages (Ada, Haskell,
Common Lisp, Pascal) already have range types.  Such range types may
help producing good code and good warnings.  The type 'idx_t' could
-   then be typedef'ed to a (signed!) range type.  */
+   then be typedef'ed to a range type that is signed after promotion.  */
 
-#if 0
-/* In the future, idx_t could be typedef'ed to a signed range type.  */
-/* Note: The clang "extended integer types", supported in Clang 11 or newer
+/* In the future, idx_t could be typedef'ed to a signed range type.
+   The clang "extended integer types", supported in Clang 11 or newer
,
are a special case of range types.  However, these types don't support binary
operators with plain integer types (e.g. expressions such as x > 1).
Therefore, they don't behave like signed types (and not like unsigned types
either).  So, we cannot use them here.  */
-typedef  idx_t;
-#else
-/* Use the signed type 'ptrdiff_t' by default.  */
+
+/* Use the signed type 'ptrdiff_t'.  */
 /* Note: ISO C does not mandate that 'size_t' and 'ptrdiff_t' have the same
-   size, but it is so an all platforms we have seen since 1990.  */
+   size, but it is so on all platforms we have seen since 1990.  */
 typedef ptrdiff_t idx_t;
-#endif
 
 /* IDX_MAX is the maximum value of an idx_t.  */
-#if defined UNSIGNED_IDX_T
-# define IDX_MAX SIZE_MAX
-#else
-# define IDX_MAX PTRDIFF_MAX
-#endif
-
-/* IDX_WIDTH is the number of bits in an idx_t (31 or 63).  */
-#define IDX_WIDTH (PTRDIFF_WIDTH - 1)
+#define IDX_MAX PTRDIFF_MAX
+
+/* So far no need has been found for an IDX_WIDTH macro.
+   Perhaps there should be another macro IDX_VALUE_BITS that does not
+   count the sign bit and is therefore one less than PTRDIFF_WIDTH.  */
 
 #endif /* _IDX_H */
-- 
2.27.0

[PATCH] free: preserve errno

[Paul Eggert]

* lib/free.c (rpl_free): Preserve errno.  Check for null only if
CANNOT_FREE_NULL is defined, as an optimization for POSIX 2008
platforms that do not preserve errno.
* m4/free.m4 (gl_FUNC_FREE): Check whether free preserves errno.
Also, define CANNOT_FREE_NULL if free cannot free NULL.
* modules/free (configure.ac): Also replace 'free' if
it does not preserve errno.
---
 ChangeLog|  9 +
 lib/free.c   | 12 ++--
 m4/free.m4   | 35 ++-
 modules/free | 12 ++--
 4 files changed, 59 insertions(+), 9 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 445e2c074..e97d82aa6 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,14 @@
 2020-12-17  Paul Eggert  
 
+   free: preserve errno
+   * lib/free.c (rpl_free): Preserve errno.  Check for null only if
+   CANNOT_FREE_NULL is defined, as an optimization for POSIX 2008
+   platforms that do not preserve errno.
+   * m4/free.m4 (gl_FUNC_FREE): Check whether free preserves errno.
+   Also, define CANNOT_FREE_NULL if free cannot free NULL.
+   * modules/free (configure.ac): Also replace 'free' if
+   it does not preserve errno.
+
idx: simplify IDX_MAX, remove IDX_WIDTH
* lib/idx.h (IDX_MAX): Simplify by removing obsolete reference
to UNSIGNED_IDX_T.
diff --git a/lib/free.c b/lib/free.c
index 50a300ffa..843ce4816 100644
--- a/lib/free.c
+++ b/lib/free.c
@@ -22,9 +22,17 @@
 
 #include 
 
+#include 
+
 void
 rpl_free (void *p)
 {
-  if (p)
-free (p);
+#ifdef CANNOT_FREE_NULL
+  if (!p)
+return;
+#endif
+
+  int err = errno;
+  free (p);
+  errno = err;
 }
diff --git a/m4/free.m4 b/m4/free.m4
index a62214cd1..62daa8ecc 100644
--- a/m4/free.m4
+++ b/m4/free.m4
@@ -35,10 +35,43 @@ AC_DEFUN([gl_FUNC_FREE],
  esac
 ])
 
+  dnl In the next release of POSIX, free must preserve errno.
+  dnl https://www.austingroupbugs.net/view.php?id=385
+  dnl https://sourceware.org/bugzilla/show_bug.cgi?id=17924
+  dnl For now, assume implementations other than glibc do not preserve errno
+  dnl unless they set _POSIX_VERSION to the next release number,
+  dnl whatever that happens to be.
+  AC_CACHE_CHECK([whether free is known to preserve errno],
+[gl_cv_func_free_preserves_errno],
+[case $host_os in
+   *-gnu* | gnu*)
+ gl_cv_func_free_preserves_errno=yes;;
+   *)
+ AC_COMPILE_IFELSE(
+   [AC_LANG_PROGRAM(
+  [[#include 
+  ]],
+  [[#if _POSIX_VERSION <= 200809
+  #error "'free' is not known to preserve errno"
+#endif
+  ]])],
+   [gl_cv_func_free_preserves_errno=yes],
+   [gl_cv_func_free_preserves_errno=no]);;
+ esac
+])
+
   if test $gl_cv_func_free = no; then
+AC_DEFINE([CANNOT_FREE_NULL], [1],
+  [Define to 1 if free (NULL) does not work.])
+  fi
+
+  case $gl_cv_func_free,$gl_cv_func_free_preserves_errno in
+   *yes,*yes) ;;
+   *)
 AC_DEFINE([free], [rpl_free],
   [Define to rpl_free if the replacement function should be used.])
-  fi
+;;
+  esac
 ])
 
 # Prerequisites of lib/free.c.
diff --git a/modules/free b/modules/free
index ec91f8889..ea2509821 100644
--- a/modules/free
+++ b/modules/free
@@ -1,8 +1,5 @@
 Description:
-Work around incompatibility on older systems where free (NULL) fails.
-
-Notice:
-This module is obsolete.
+Work around systems where free sets errno or free (NULL) fails.
 
 Files:
 lib/free.c
@@ -12,10 +9,13 @@ Depends-on:
 
 configure.ac:
 gl_FUNC_FREE
-if test $gl_cv_func_free = no; then
+case $gl_cv_func_free,$gl_cv_func_free_errno in
+ *yes,*yes) ;;
+ *)
   AC_LIBOBJ([free])
   gl_PREREQ_FREE
-fi
+  ;;
+esac
 
 Makefile.am:
 
-- 
2.27.0