Re: Make 3.99.91/92 - Output-Synchronization fails with UAC or Non-Admins (under Windows)

[Eli Zaretskii]

> Date: Mon, 23 Sep 2013 16:21:30 +0200
> From: Markus Eisenmann <address@hidden>
> 
> I have built a windows-make version of the release-canditate versions
> 3.99.91 and 3.99.92,
> using Visual Studio and MinGW. The result, I.e. make fails if I'm using
> parallel builds and the feature  'output-synchronization' with an error
> like 'could not create tmpfile'.
> 
> After spending some time for debugging, I have detected the "problem":
> - A temporary file is used to buffer the outputs (of course)
> - this file is created with calling 'tmpfile()'.
> 
> As written in
> http://msdn.microsoft.com/en-us/library/x8x7sakw%28v=vs.90%29.aspx,
> tmpfile() does create a temporary file in the root directory; But in many
> cases - e.g. inhibited by NTFS-security or if UAC is active - a creation of
> files in the root-directory is forbidden.

I don't think this has anything to do with UAC, it's just that the
root of a system disk is write-protected from casual writes.  How
silly of Microsoft not to do something about this on Windows 7 and
later!

> IMHO, I think, this type of temporary files should be created like other
> needed temporary (batch-) files in the TMP/TEMP-folder (using the
> associated environment variable).

I agree.  Can you please test with the following patch?  I'm also
testing it; if it will give good results, I will commit it to the
repository.

Thanks.

--- output.c~0  2013-09-23 00:10:34.000000000 +0300
+++ output.c    2013-09-23 22:58:51.963375000 +0300
@@ -72,6 +72,103 @@ msc_vsnprintf (char *str, size_t size, c
 }
 #endif
 
+#ifdef WINDOWS32
+/* A replacement for tmpfile, since the MSVCRT implementation creates
+   the file in the root directory of the current drive, which might
+   not be writable by our user.  Most of the code borrowed from
+   create_batch_file, see job.c.  */
+FILE *
+tmpfile (void)
+{
+  char temp_path[MAXPATHLEN];
+  unsigned path_size = GetTempPath (sizeof temp_path, temp_path);
+  int path_is_dot = 0;
+  /* The following variable is static so we won't try to reuse a name
+     that was generated a little while ago, because that file might
+     not be on disk yet, since we use FILE_ATTRIBUTE_TEMPORARY below,
+     which tells the OS it doesn't need to flush the cache to disk.
+     If the file is not yet on disk, we might think the name is
+     available, while it really isn't.  This happens in parallel
+     builds, where Make doesn't wait for one job to finish before it
+     launches the next one.  */
+  static unsigned uniq = 0;
+  static int second_loop = 0;
+  const char base[] = "make_tmpf";
+  const unsigned sizemax = sizeof base - 1 + 4 + 10 + 10;
+  unsigned pid = GetCurrentProcessId ();
+
+  if (path_size == 0)
+    {
+      path_size = GetCurrentDirectory (sizeof temp_path, temp_path);
+      path_is_dot = 1;
+    }
+
+  ++uniq;
+  if (uniq >= 0x10000 && !second_loop)
+    {
+      /* If we already had 64K batch files in this
+         process, make a second loop through the numbers,
+         looking for free slots, i.e. files that were
+         deleted in the meantime.  */
+      second_loop = 1;
+      uniq = 1;
+    }
+  while (path_size > 0 &&
+         path_size + sizemax < sizeof temp_path &&
+         !(uniq >= 0x10000 && second_loop))
+    {
+      sprintf (temp_path + path_size,
+              "%s%s%u-%x.tmp",
+              temp_path[path_size - 1] == '\\' ? "" : "\\",
+              base, pid, uniq);
+      /* O_CREAT | O_EXCL | O_RDWR | O_BINARY | O_TEMPORARY
+        SH_DENYNO */
+      HANDLE h = CreateFile (temp_path,  /* file name */
+                             GENERIC_READ | GENERIC_WRITE | DELETE, /* desired 
access */
+                             0,                            /* no share mode */
+                             NULL,                         /* default security 
attributes */
+                             CREATE_NEW,                   /* creation 
disposition */
+                             FILE_ATTRIBUTE_NORMAL |       /* flags and 
attributes */
+                             FILE_ATTRIBUTE_TEMPORARY |
+                            FILE_FLAG_DELETE_ON_CLOSE,
+                             NULL);                        /* no template file 
*/
+
+      if (h == INVALID_HANDLE_VALUE)
+        {
+          const DWORD er = GetLastError ();
+
+          if (er == ERROR_FILE_EXISTS || er == ERROR_ALREADY_EXISTS)
+            {
+              ++uniq;
+              if (uniq == 0x10000 && !second_loop)
+                {
+                  second_loop = 1;
+                  uniq = 1;
+                }
+            }
+
+          /* the temporary path is not guaranteed to exist */
+          else if (path_is_dot == 0)
+            {
+              path_size = GetCurrentDirectory (sizeof temp_path, temp_path);
+              path_is_dot = 1;
+            }
+
+          else
+           break;
+        }
+      else
+        {
+          int fd = _open_osfhandle ((intptr_t)h, 0);
+
+          return _fdopen (fd, "w+b");
+        }
+    }
+
+  return NULL;
+}
+#endif
+
 /* Write a string to the current STDOUT or STDERR.  */
 static void
 _outputs (struct output *out, int is_err, const char *msg)