On Sun, Feb 10, 2019 at 03:55:41PM +0100, Marc Espie wrote:
> Something similar to this  perhaps ?
> Not fully tested yet, but it should avoid the race of trying to 
> unlink tempfile several times, and also fix the file name in error messages.

That's now been tested (gone thru a build + xbuild and bulk in progress
that went thru go-bootstrap and go without issues).

Okay ?

This needs re-applying Ingo's patch first, of course, which I can either do
or we coordinate with Ingo.



Index: xinstall.c
===================================================================
RCS file: /cvs/src/usr.bin/xinstall/xinstall.c,v
retrieving revision 1.68
diff -u -p -r1.68 xinstall.c
--- xinstall.c  8 Feb 2019 12:53:44 -0000       1.68
+++ xinstall.c  10 Feb 2019 14:53:49 -0000
@@ -222,6 +222,7 @@ install(char *from_name, char *to_name, 
        struct timespec ts[2];
        int devnull, from_fd, to_fd, serrno, files_match = 0;
        char *p;
+       char *target_name = tempfile;
 
        (void)memset((void *)&from_sb, 0, sizeof(from_sb));
        (void)memset((void *)&to_sb, 0, sizeof(to_sb));
@@ -311,10 +312,14 @@ install(char *from_name, char *to_name, 
                        } else {
                                files_match = 1;
                                (void)unlink(tempfile);
+                               target_name = to_name;
+                               (void)close(temp_fd);
                        }
                }
-               (void)close(to_fd);
-               to_fd = temp_fd;
+               if (!files_match) {
+                       (void)close(to_fd);
+                       to_fd = temp_fd;
+               }
        }
 
        /*
@@ -333,13 +338,15 @@ install(char *from_name, char *to_name, 
        if ((gid != (gid_t)-1 || uid != (uid_t)-1) &&
            fchown(to_fd, uid, gid)) {
                serrno = errno;
-               (void)unlink(tempfile);
-               errx(1, "%s: chown/chgrp: %s", tempfile, strerror(serrno));
+               if (target_name == tempfile)
+                       (void)unlink(tempfile);
+               errx(1, "%s: chown/chgrp: %s", target_name, strerror(serrno));
        }
        if (fchmod(to_fd, mode)) {
                serrno = errno;
-               (void)unlink(tempfile);
-               errx(1, "%s: chmod: %s", tempfile, strerror(serrno));
+               if (target_name == tempfile)
+                       (void)unlink(tempfile);
+               errx(1, "%s: chmod: %s", target_name, strerror(serrno));
        }
 
        /*
@@ -349,7 +356,7 @@ install(char *from_name, char *to_name, 
        if (fchflags(to_fd,
            flags & SETFLAGS ? fset : from_sb.st_flags & ~UF_NODUMP)) {
                if (errno != EOPNOTSUPP || (from_sb.st_flags & ~UF_NODUMP) != 0)
-                       warnx("%s: chflags: %s", tempfile, strerror(errno));
+                       warnx("%s: chflags: %s", target_name, strerror(errno));
        }
 
        if (flags & USEFSYNC)

Reply via email to