From 1bc68cb4d5ea80c2d8f626e059a57c2280d9d663 Mon Sep 17 00:00:00 2001 From: Paul Eggert Date: Sat, 2 Jul 2022 16:18:07 -0500 Subject: sed: fix temp file cleanup MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Without this fix, the code would sometimes use FP after calling fclose (FP), which has undefined behavior in C. Problem found with --enable-gcc-warnings and GCC 12. * sed/execute.c (open_next_file): Do not register here, as it’s too late and this can cause the file to not be cleaned up. * sed/sed.c (G_file_to_unlink, register_cleanup_file, cancel_cleanup): Move from here to utils.c. (cleanup): Call remove_cleanup_file instead of doing it by hand. * sed/utils.c (struct open_file): Remove member temp (which was always false) and fclose_failed (which was not enough to prevent calling fclose with a bad pointer). All uses changed. (register_open_file): Do not access p->fp after it’s fclosed, as that has undefined behavior in C. Use xmalloc instead of xcalloc, since we initialize all members. (G_file_to_unlink, register_cleanup_file, cancel_cleanup): Move from utils.c to here. (remove_cleanup_file): New function. (ck_mkstemp): Fix a screwup when mkostemp succeeded but set_binary_mode or fdopen failed: we might misuse a null pointer, or forget to clean up the newly-created temp file. (ck_getdelim): Rename local to avoid confusion with global. (mark_as_fclose_failed): Remove. All uses removed. (ck_fclose): Remove entry from open_files before attempting to fclose it, so that panicking doesn’t try to fclose it again. (do_ck_fclose): New arg NAME so that there’s no need to call mark_as_fclose_failed, which inspected FP after fclosing it, which is undefined behavior. (ck_rename): Omit arg UNLINK_IF_FAIL. All callers changed. The cleanup handler removes this file now, as needed. --- sed/utils.h | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'sed/utils.h') diff --git a/sed/utils.h b/sed/utils.h index e3a8532..cac8a05 100644 --- a/sed/utils.h +++ b/sed/utils.h @@ -40,11 +40,14 @@ size_t ck_getdelim (char **text, size_t *buflen, char buffer_delimiter, FILE *stream); FILE * ck_mkstemp (char **p_filename, const char *tmpdir, const char *base, const char *mode) _GL_ARG_NONNULL ((1, 2, 3, 4)); -void ck_rename (const char *from, const char *to, const char *unlink_if_fail); +void ck_rename (const char *from, const char *to); void *ck_malloc (size_t size); void *ck_realloc (void *ptr, size_t size); +void cancel_cleanup (void); +void remove_cleanup_file (void); + struct buffer *init_buffer (void); char *get_buffer (struct buffer const *b) _GL_ATTRIBUTE_PURE; size_t size_buffer (struct buffer const *b) _GL_ATTRIBUTE_PURE; -- cgit v1.2.1