* Re: master 7cd8236d35c: Pacify --enable-gcc-warnings with emacs_fdopen
2023-08-08 1:07 ` master 7cd8236d35c: Pacify --enable-gcc-warnings with emacs_fdopen Po Lu
@ 2023-08-08 4:29 ` Paul Eggert
2023-08-08 5:26 ` Po Lu
0 siblings, 1 reply; 4+ messages in thread
From: Paul Eggert @ 2023-08-08 4:29 UTC (permalink / raw)
To: Po Lu, emacs-devel
[-- Attachment #1: Type: text/plain, Size: 1043 bytes --]
On 2023-08-07 18:07, Po Lu wrote:
> now I receive warnings during the automated daily build of
> the Android port:
>
> ../../emacs/src/xfaces.c: In function ‘Fx_load_color_file’:
> ../../emacs/src/xfaces.c:7007:7: warning: ‘emacs_fclose’ called on pointer returned from a mismatched allocation function [-Wmismatched-dealloc]
> 7007 | emacs_fclose (fp);
> | ^~~~~~~~~~~~~~~~~
> ../../emacs/src/xfaces.c:6984:8: note: returned from ‘emacs_fopen’
> 6984 | fp = emacs_fopen (SSDATA (abspath), "r" FOPEN_TEXT);
I gave a shot at fixing that by installing the attached. It works for me
on Fedora 38, but then again the old version worked too. If this doesn't
work on Android, it might help to send the preprocessed output.
By the way, I see lisp.h includes stdio.h now. Would it make sense to
create a new include file for use only by the parts of Emacs that need
to deal with stdio streams? We could move some of the stuff out of
lisp.h, which is bulging a bit at the seams nowadays.
[-- Attachment #2: 0001-Fix-some-emacs_fopen-confusion.patch --]
[-- Type: text/x-patch, Size: 4314 bytes --]
From 85b6c150c8a356ebf3442fde2836364954aa938c Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Mon, 7 Aug 2023 21:23:28 -0700
Subject: [PATCH] Fix some emacs_fopen confusion
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Problem reported by Po Lu in:
https://lists.gnu.org/r/emacs-devel/2023-08/msg00195.html
* src/comp.c (comp_hash_source_file, Fcomp__release_ctxt):
* src/sysdep.c (get_up_time, procfs_ttyname, procfs_get_total_memory):
Be more systematic about using emacs_fclose on streams that were
opened with emacs_fopen or emacs_fdopen. Do this even if not
Android, as this simplifies checking that it's done consistently.
* src/lisp.h (emacs_fclose): If it’s just fclose,
make it a macro rather than a function, to avoid confusing gcc
-Wmismatched-dealloc.
(emacs_fopen): Move decl here from sysstdio.h, because sysstdio.h
is included from non-Emacs executables and emacs_fopen is good
only inside Emacs.
* src/sysdep.c (emacs_fclose): Define as a function only if Android.
---
src/comp.c | 4 ++--
src/lisp.h | 6 ++++++
src/sysdep.c | 12 +++++-------
src/sysstdio.h | 2 --
4 files changed, 13 insertions(+), 11 deletions(-)
diff --git a/src/comp.c b/src/comp.c
index 1bde4ae5821..b81a80b00f8 100644
--- a/src/comp.c
+++ b/src/comp.c
@@ -776,7 +776,7 @@ comp_hash_source_file (Lisp_Object filename)
#else
int res = md5_stream (f, SSDATA (digest));
#endif
- fclose (f);
+ emacs_fclose (f);
if (res)
xsignal2 (Qfile_notify_error, build_string ("hashing failed"), filename);
@@ -4749,7 +4749,7 @@ DEFUN ("comp--release-ctxt", Fcomp__release_ctxt, Scomp__release_ctxt,
gcc_jit_context_release (comp.ctxt);
if (logfile)
- fclose (logfile);
+ emacs_fclose (logfile);
comp.ctxt = NULL;
return Qt;
diff --git a/src/lisp.h b/src/lisp.h
index 85de57b0b2f..2f26e5eddce 100644
--- a/src/lisp.h
+++ b/src/lisp.h
@@ -5086,9 +5086,15 @@ maybe_disable_address_randomization (int argc, char **argv)
extern int emacs_open_noquit (const char *, int, int);
extern int emacs_pipe (int[2]);
extern int emacs_close (int);
+#if !(defined HAVE_ANDROID && !defined ANDROID_STUBIFY)
+# define emacs_fclose fclose
+#else
extern int emacs_fclose (FILE *);
+#endif
extern FILE *emacs_fdopen (int, const char *)
ATTRIBUTE_MALLOC ATTRIBUTE_DEALLOC (emacs_fclose, 1);
+extern FILE *emacs_fopen (char const *, char const *)
+ ATTRIBUTE_MALLOC ATTRIBUTE_DEALLOC (emacs_fclose, 1);
extern int emacs_unlink (const char *);
extern int emacs_symlink (const char *, const char *);
extern int emacs_rmdir (const char *);
diff --git a/src/sysdep.c b/src/sysdep.c
index a995bc66741..0f8b70c8248 100644
--- a/src/sysdep.c
+++ b/src/sysdep.c
@@ -2644,15 +2644,13 @@ emacs_fdopen (int fd, const char *mode)
clear information associated with the FILE's file descriptor if
necessary. */
+#if defined HAVE_ANDROID && !defined ANDROID_STUBIFY
int
emacs_fclose (FILE *stream)
{
-#if !(defined HAVE_ANDROID && !defined ANDROID_STUBIFY)
- return fclose (stream);
-#else
return android_fclose (stream);
-#endif
}
+#endif
/* Wrappers around unlink, symlink, rename, renameat_noreplace, and
rmdir. These operations handle asset and content directories on
@@ -3492,7 +3490,7 @@ get_up_time (void)
Lisp_Object subsec = Fcons (make_fixnum (upfrac), make_fixnum (hz));
up = Ftime_add (sec, subsec);
}
- fclose (fup);
+ emacs_fclose (fup);
}
unblock_input ();
@@ -3540,7 +3538,7 @@ procfs_ttyname (int rdev)
}
}
}
- fclose (fdev);
+ emacs_fclose (fdev);
}
unblock_input ();
return build_string (name);
@@ -3582,7 +3580,7 @@ procfs_get_total_memory (void)
}
while (!done);
- fclose (fmem);
+ emacs_fclose (fmem);
}
unblock_input ();
return retval;
diff --git a/src/sysstdio.h b/src/sysstdio.h
index 5a973c833cc..8e9e5bec86c 100644
--- a/src/sysstdio.h
+++ b/src/sysstdio.h
@@ -28,8 +28,6 @@ #define EMACS_SYSSTDIO_H
#include <attribute.h>
#include <unlocked-io.h>
-extern FILE *emacs_fopen (char const *, char const *)
- ATTRIBUTE_MALLOC ATTRIBUTE_DEALLOC (fclose, 1);
extern void errputc (int);
extern void errwrite (void const *, ptrdiff_t);
extern void close_output_streams (void);
--
2.39.2
^ permalink raw reply related [flat|nested] 4+ messages in thread