Re: ptrdiff_t overflow checks for malloc-posix etc.

[Bruno Haible]

Hi Paul,

On 2021-04-18 you replied:
> >   if test $REPLACE_REALLOC = 0; then
> >     _AC_FUNC_REALLOC_IF([], [REPLACE_REALLOC=1])
> >   fi
> 
> I confess I don't like the style as much: it makes the shell code a bit 
> less readable, at least to me. But it appears that this style isn't 
> needed anyway.

Sorry, but the style

   test $REPLACE_REALLOC = 1 || <some big macro invocation>

is buggy. Let's take, as example, the current GNU m4 snapshot. Its
configuration produces output like this:

  ...
  checking for libsigsegv... yes
  checking how to link with libsigsegv... /inst-x86_64-x32/lib/libsigsegv.a
  yes
  checking whether this system supports file names of any length... no
  ...
  checking whether readlink truncates results correctly... yes
  yes
  checking for reallocarray... no
  ...

(or 'no' instead of 'yes' on some non-glibc systems).

When I look into the generated configure file I see this for the first
lonely 'yes':

--------------------------------------------------------------------------------
  test $REPLACE_CALLOC = 1 ||
     { printf "%s\n" "$as_me:${as_lineno-$LINENO}: checking whether calloc (0, 
n) and calloc (n, 0) return nonnull" >&5
printf %s "checking whether calloc (0, n) and calloc (n, 0) return nonnull... " 
>&6; }
if test ${ac_cv_func_calloc_0_nonnull+y}
...
fi
{ printf "%s\n" "$as_me:${as_lineno-$LINENO}: result: 
$ac_cv_func_calloc_0_nonnull" >&5
printf "%s\n" "$ac_cv_func_calloc_0_nonnull" >&6; }
  case $ac_cv_func_calloc_0_nonnull in #(
  *yes) :
     ;; #(
  *) :
    REPLACE_CALLOC=1 ;;
esac
--------------------------------------------------------------------------------

and this for the second lonely 'yes':

--------------------------------------------------------------------------------
  test $REPLACE_REALLOC = 1 ||
    { printf "%s\n" "$as_me:${as_lineno-$LINENO}: checking whether realloc (0, 
0) returns nonnull" >&5
printf %s "checking whether realloc (0, 0) returns nonnull... " >&6; }
if test ${ac_cv_func_realloc_0_nonnull+y}
...
{ printf "%s\n" "$as_me:${as_lineno-$LINENO}: result: 
$ac_cv_func_realloc_0_nonnull" >&5
printf "%s\n" "$ac_cv_func_realloc_0_nonnull" >&6; }
  case $ac_cv_func_realloc_0_nonnull in #(
  *yes) :
     ;; #(
  *) :
    REPLACE_REALLOC=1 ;;
esac
--------------------------------------------------------------------------------

As you can see, this logic has caused the first two lines after the 'test'
to be skipped, but the next lines were executed although they shouldn't.

So, not only the configure output was wrong; also the logic which statements
of the configure file get executed was wrong.

I have to commit this fix. The alternative, to force some macros
expand into a statement group enclosed by { ... }, would be fragile.
With this fix, the configure output looks right:

  ...
  checking for libsigsegv... yes
  checking how to link with libsigsegv... /inst-x86_64-x32/lib/libsigsegv.a
  checking whether calloc (0, n) and calloc (n, 0) return nonnull... yes
  checking whether this system supports file names of any length... no
  ...
  checking whether readlink truncates results correctly... yes
  checking whether realloc (0, 0) returns nonnull... yes
  checking for reallocarray... no
  ...


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

        {malloc,realloc,calloc}-gnu: Fix autoconf macro (regression 2021-04-18).
        * m4/malloc.m4 (gl_FUNC_MALLOC_GNU): Don't assume that
        _AC_FUNC_MALLOC_IF expands to a single shell statement.
        * m4/realloc.m4 (gl_FUNC_REALLOC_GNU): Don't assume that
        _AC_FUNC_REALLOC_IF expands to a single shell statement.
        * m4/calloc.m4 (gl_FUNC_CALLOC_GNU): Don't assume that
        _AC_FUNC_CALLOC_IF expands to a single shell statement.

diff --git a/m4/calloc.m4 b/m4/calloc.m4
index 2f0abee..7575a69 100644
--- a/m4/calloc.m4
+++ b/m4/calloc.m4
@@ -1,4 +1,4 @@
-# calloc.m4 serial 25
+# calloc.m4 serial 26
 
 # Copyright (C) 2004-2021 Free Software Foundation, Inc.
 # This file is free software; the Free Software Foundation
@@ -58,7 +58,9 @@ AC_DEFUN([gl_FUNC_CALLOC_GNU],
 [
   AC_REQUIRE([gl_STDLIB_H_DEFAULTS])
   AC_REQUIRE([gl_FUNC_CALLOC_POSIX])
-  test $REPLACE_CALLOC = 1 || _AC_FUNC_CALLOC_IF([], [REPLACE_CALLOC=1])
+  if test $REPLACE_CALLOC = 0; then
+    _AC_FUNC_CALLOC_IF([], [REPLACE_CALLOC=1])
+  fi
 ])# gl_FUNC_CALLOC_GNU
 
 # gl_FUNC_CALLOC_POSIX
diff --git a/m4/malloc.m4 b/m4/malloc.m4
index de1b2c6..6fcd4ad 100644
--- a/m4/malloc.m4
+++ b/m4/malloc.m4
@@ -1,4 +1,4 @@
-# malloc.m4 serial 25
+# malloc.m4 serial 26
 dnl Copyright (C) 2007, 2009-2021 Free Software Foundation, Inc.
 dnl This file is free software; the Free Software Foundation
 dnl gives unlimited permission to copy and/or distribute it,
@@ -43,7 +43,9 @@ AC_DEFUN([gl_FUNC_MALLOC_GNU],
 [
   AC_REQUIRE([gl_STDLIB_H_DEFAULTS])
   AC_REQUIRE([gl_FUNC_MALLOC_POSIX])
-  test $REPLACE_MALLOC = 1 || _AC_FUNC_MALLOC_IF([], [REPLACE_MALLOC=1])
+  if test $REPLACE_MALLOC = 0; then
+    _AC_FUNC_MALLOC_IF([], [REPLACE_MALLOC=1])
+  fi
 ])
 
 # gl_FUNC_MALLOC_PTRDIFF
diff --git a/m4/realloc.m4 b/m4/realloc.m4
index 8eb6b19..9925917 100644
--- a/m4/realloc.m4
+++ b/m4/realloc.m4
@@ -1,4 +1,4 @@
-# realloc.m4 serial 22
+# realloc.m4 serial 23
 dnl Copyright (C) 2007, 2009-2021 Free Software Foundation, Inc.
 dnl This file is free software; the Free Software Foundation
 dnl gives unlimited permission to copy and/or distribute it,
@@ -43,7 +43,9 @@ AC_DEFUN([gl_FUNC_REALLOC_GNU],
 [
   AC_REQUIRE([gl_STDLIB_H_DEFAULTS])
   AC_REQUIRE([gl_FUNC_REALLOC_POSIX])
-  test $REPLACE_REALLOC = 1 || _AC_FUNC_REALLOC_IF([], [REPLACE_REALLOC=1])
+  if test $REPLACE_REALLOC = 0; then
+    _AC_FUNC_REALLOC_IF([], [REPLACE_REALLOC=1])
+  fi
 ])# gl_FUNC_REALLOC_GNU
 
 # gl_FUNC_REALLOC_POSIX