[bug #63157] Unlink temporary files.

[Dmitry Goncharov]

Follow-up Comment #11, bug #63157 (project make):

> If temp_stdin is fclosed, then there's no need to call close on its fileno.

Right. We have to take care of the critical section between fopen and fclose.

The reasoning is the following.
1. The patch works on unices.
2. On windows the patch works if the file is closed.
3. eval_makefile uses fopen and then fclose to open and close the file.
4. On windowns the file is not deleted, if a signal arrives after the file is
opened and before the file is closed by eval_makefile.

One option to handle this critical section on windows is to set the fd of the
file to a file scope variable and then close this fd in temp_stdin_unlink.
Something like this (Not tested on windows).


++++
diff --git a/src/main.c b/src/main.c
index 129dd661..cf87a6a0 100644
--- a/src/main.c
+++ b/src/main.c
@@ -306,6 +306,7 @@ unsigned long command_count = 1;
 /* Remember the location of the name of the batch file from stdin.  */

 static int stdin_offset = -1;
+static int temp_stdin_fileno = -1;

 ^L
 /* The usage output.  We write it this way to make life easier for the
@@ -3714,12 +3715,33 @@ clean_jobserver (int status)
     }
 }

+void
+temp_stdin_set_fileno (const char *name, FILE *f)
+{
+  if (temp_stdin_fileno == -1 && f &&
+      stdin_offset >= 0 && streq (makefiles->list[stdin_offset], name))
+    temp_stdin_fileno = fileno (f);
+}
+
+void
+temp_stdin_reset_fileno (const char *name)
+{
+  if (temp_stdin_fileno >= 0 &&
+      stdin_offset >= 0 && streq (makefiles->list[stdin_offset], name))
+    temp_stdin_fileno = -1;
+}
+
 void
 temp_stdin_unlink ()
 {
   /* This function is called from a signal handler.
    * Keep async-signal-safe.  */
   /* Get rid of a temp file from reading a makefile from stdin.  */
+#ifdef WINDOWS32
+  /* Windows needs the file to be closed for unlink to succeed.  */
+  if (temp_stdin_fileno >= 0 && close (temp_stdin_fileno) == 0)
+    temp_stdin_fileno = -1;
+#endif
   if (stdin_offset >= 0 && unlink (makefiles->list[stdin_offset]) == 0)
     stdin_offset = -1;
 }
diff --git a/src/makeint.h b/src/makeint.h
index d0e0fab3..cf2975c6 100644
--- a/src/makeint.h
+++ b/src/makeint.h
@@ -530,6 +530,8 @@ void out_of_memory () NORETURN;
                                  (_f), (_n), (_s))

 void decode_env_switches (const char*, size_t line);
+void temp_stdin_set_fileno (const char *name, FILE *f);
+void temp_stdin_reset_fileno (const char *name);
 void temp_stdin_unlink ();
 void die (int) NORETURN;
 void pfatal_with_name (const char *) NORETURN;
diff --git a/src/read.c b/src/read.c
index 79315503..b8e7baed 100644
--- a/src/read.c
+++ b/src/read.c
@@ -353,6 +353,7 @@ eval_makefile (const char *filename, unsigned short
flags)
   errno = 0;
   ENULLLOOP (ebuf.fp, fopen (filename, "r"));
   deps->error = errno;
+  temp_stdin_set_fileno (filename, ebuf.fp);

   /* Check for unrecoverable errors: out of mem or FILE slots.  */
   switch (deps->error)
@@ -441,6 +442,7 @@ eval_makefile (const char *filename, unsigned short
flags)
   reading_file = curfile;

   fclose (ebuf.fp);
+  temp_stdin_reset_fileno (filename);

   free (ebuf.bufstart);
   free_alloca ();
----

There is still a smaller critical section. If a signal arrives between fopen
and temp_stdin_set_fileno, the file is not deleted. Signals can be blocked
during this smaller critical section. But i think, that's an overkill.

> What do you mean by "We won't be able to use fclose, which is okay, since
make is exiting."?

Inside temp_stdin_unlink make can only do async signal safe calls. This rules
out fclose. Make can call close, because close is async signal safe. Calling
close, rather than fclose, leaves the internal FILE bookeeping messed up,
which is okay, because make is exiting.


    _______________________________________________________

Reply to this item at:

  <https://savannah.gnu.org/bugs/?63157>

_______________________________________________
Message sent via Savannah
https://savannah.gnu.org/