Paul Eggert wrote:
> diff --git a/m4/fclose.m4 b/m4/fclose.m4
> index dc021dd9f3..74b7df0a77 100644
> --- a/m4/fclose.m4
> +++ b/m4/fclose.m4
> @@ -31,6 +31,10 @@ AC_DEFUN([gl_FUNC_FCLOSE],
> *) REPLACE_FCLOSE=1 ;;
> esac
> fi
> +
> + if test $REPLACE_FCLOSE = 1; then
> + REPLACE_FOPEN=1
> + fi
> ])
>
...
> diff --git a/modules/fclose b/modules/fclose
> index 703de9e686..9a949e48b8 100644
> --- a/modules/fclose
> +++ b/modules/fclose
> @@ -9,12 +9,18 @@ Depends-on:
...
> configure.ac:
> gl_FUNC_FCLOSE
> +if test $REPLACE_FCLOSE = 1; then
> + REPLACE_FOPEN=1
> + AC_LIBOBJ([fopen])
> + gl_PREREQ_FOPEN
> +fi
> gl_CONDITIONAL([GL_COND_OBJ_FCLOSE], [test $REPLACE_FCLOSE = 1])
> gl_STDIO_MODULE_INDICATOR([fclose])
This patch is technically correct, but has maintainability problems: It
intermingles the actions of the modules 'fopen' and 'fclose'. In particular:
* Setting the variable REPLACE_FOO=1 from elsewhere than m4/foo.m4 is
not a good practice, because
(1) It is likely to lead to bugs, depending on the order in which the
various gl_FUNC_* macros are expanded.
(2) It is likely to lead to a bug the next time module 'foo' is being
modified.
* Invoking AC_LIBOBJ([foo]) outside of module 'foo' is not a good practice,
because it makes it hard or impossible to understand the conditions
when foo.c gets compiled. We have already two modules 'fopen' and
'fopen-gnu' that do AC_LIBOBJ([foo]); this is hard enough to understand.
The fact that you needed to duplicate the logic of
if test $REPLACE_FCLOSE = 1; then
REPLACE_FOPEN=1
is an indication of a workaround to problem (1).
For solving problem (1), Autoconf offers us AC_REQUIRE; so that's what we
should use.
It is reasonably maintainable, on the other hand, to have a module depend
on the results of the configuration of another module, that is, to *inspect*
the value of REPLACE_FOO of module 'foo'. For example,
- fclose.m4 does AC_REQUIRE([gl_FUNC_CLOSE]) and then tests REPLACE_CLOSE.
That is fine.
- fclose.m4 calls gl_FUNC_FFLUSH_STDIN, which exists in fflush.m4. That
is also fine.
I'm therefore moving the "if REPLACE_FCLOSE is 1, set REPLACE_FOPEN to 1"
logic to module 'fopen'. You'll notice that the logic of
if test $REPLACE_FCLOSE = 1; then
REPLACE_FOPEN=1
is now present in a single place only.
2023-04-26 Bruno Haible <br...@clisp.org>
fclose: Make last change more maintainable.
* m4/fclose.m4 (gl_FUNC_FCLOSE): Define through AC_DEFUN_ONCE. Don't
modify REPLACE_FOPEN.
* modules/fclose (Depends-on): Add comment.
(configure.ac): Don't modify REPLACE_FOPEN. Don't duplicate actions of
module 'fopen'.
* m4/fopen.m4 (gl_FUNC_FOPEN_ITSELF): Renamed from gl_FUNC_FOPEN.
(gl_FUNC_FOPEN): New macro.
* modules/fopen (Files): Add m4/fclose.m4, m4/fflush.m4.
* m4/close.m4 (gl_FUNC_CLOSE): Define through AC_DEFUN_ONCE.
diff --git a/m4/close.m4 b/m4/close.m4
index 9f95c670e5..0feabd6917 100644
--- a/m4/close.m4
+++ b/m4/close.m4
@@ -1,10 +1,10 @@
-# close.m4 serial 9
+# close.m4 serial 10
dnl Copyright (C) 2008-2023 Free Software Foundation, Inc.
dnl This file is free software; the Free Software Foundation
dnl gives unlimited permission to copy and/or distribute it,
dnl with or without modifications, as long as this notice is preserved.
-AC_DEFUN([gl_FUNC_CLOSE],
+AC_DEFUN_ONCE([gl_FUNC_CLOSE],
[
AC_REQUIRE([gl_UNISTD_H_DEFAULTS])
m4_ifdef([gl_MSVC_INVAL], [
diff --git a/m4/fclose.m4 b/m4/fclose.m4
index 74b7df0a77..e9291f0bda 100644
--- a/m4/fclose.m4
+++ b/m4/fclose.m4
@@ -1,10 +1,10 @@
-# fclose.m4 serial 10
+# fclose.m4 serial 11
dnl Copyright (C) 2008-2023 Free Software Foundation, Inc.
dnl This file is free software; the Free Software Foundation
dnl gives unlimited permission to copy and/or distribute it,
dnl with or without modifications, as long as this notice is preserved.
-AC_DEFUN([gl_FUNC_FCLOSE],
+AC_DEFUN_ONCE([gl_FUNC_FCLOSE],
[
AC_REQUIRE([gl_STDIO_H_DEFAULTS])
AC_REQUIRE([AC_CANONICAL_HOST])
@@ -31,10 +31,6 @@ AC_DEFUN([gl_FUNC_FCLOSE]
*) REPLACE_FCLOSE=1 ;;
esac
fi
-
- if test $REPLACE_FCLOSE = 1; then
- REPLACE_FOPEN=1
- fi
])
dnl Determine whether fclose works on input streams.
diff --git a/m4/fopen.m4 b/m4/fopen.m4
index 6806394f8c..7daa4caec5 100644
--- a/m4/fopen.m4
+++ b/m4/fopen.m4
@@ -1,10 +1,10 @@
-# fopen.m4 serial 14
+# fopen.m4 serial 15
dnl Copyright (C) 2007-2023 Free Software Foundation, Inc.
dnl This file is free software; the Free Software Foundation
dnl gives unlimited permission to copy and/or distribute it,
dnl with or without modifications, as long as this notice is preserved.
-AC_DEFUN([gl_FUNC_FOPEN],
+AC_DEFUN([gl_FUNC_FOPEN_ITSELF],
[
AC_REQUIRE([gl_STDIO_H_DEFAULTS])
AC_REQUIRE([AC_CANONICAL_HOST])
@@ -58,6 +58,15 @@ AC_DEFUN([gl_FUNC_FOPEN]
esac
])
+AC_DEFUN([gl_FUNC_FOPEN],
+[
+ AC_REQUIRE([gl_FUNC_FOPEN_ITSELF])
+ AC_REQUIRE([gl_FUNC_FCLOSE])
+ if test $REPLACE_FCLOSE = 1; then
+ REPLACE_FOPEN=1
+ fi
+])
+
AC_DEFUN([gl_FUNC_FOPEN_GNU],
[
AC_REQUIRE([gl_FUNC_FOPEN])
diff --git a/modules/fclose b/modules/fclose
index 9a949e48b8..0ef34bdfd9 100644
--- a/modules/fclose
+++ b/modules/fclose
@@ -9,18 +9,16 @@ Depends-on:
stdio
close [test $REPLACE_FCLOSE = 1]
fflush [test $REPLACE_FCLOSE = 1]
-fopen [test $REPLACE_FCLOSE = 1]
freading [test $REPLACE_FCLOSE = 1]
lseek [test $REPLACE_FCLOSE = 1]
msvc-inval [test $REPLACE_FCLOSE = 1]
+# The code of fclose does not need to call fopen. But when gcc's '-fanalyzer'
+# option is in use and stdio.h does '#define fclose rpl_fclose', stdio.h also
+# needs to do '#define fopen rpl_fopen', otherwise a warning may result.
+fopen [test $REPLACE_FCLOSE = 1]
configure.ac:
gl_FUNC_FCLOSE
-if test $REPLACE_FCLOSE = 1; then
- REPLACE_FOPEN=1
- AC_LIBOBJ([fopen])
- gl_PREREQ_FOPEN
-fi
gl_CONDITIONAL([GL_COND_OBJ_FCLOSE], [test $REPLACE_FCLOSE = 1])
gl_STDIO_MODULE_INDICATOR([fclose])
diff --git a/modules/fopen b/modules/fopen
index da216c9cec..3d538393f1 100644
--- a/modules/fopen
+++ b/modules/fopen
@@ -4,6 +4,8 @@ fopen() function: open a stream to a file.
Files:
lib/fopen.c
m4/fopen.m4
+m4/fclose.m4
+m4/fflush.m4
Depends-on:
stdio
