diff --git a/usr.bin/xinstall/install.1 b/usr.bin/xinstall/install.1 --- a/usr.bin/xinstall/install.1 +++ b/usr.bin/xinstall/install.1 @@ -25,7 +25,7 @@ .\" OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF .\" SUCH DAMAGE. .\" -.Dd August 4, 2022 +.Dd April 10, 2024 .Dt INSTALL 1 .Os .Sh NAME @@ -225,16 +225,18 @@ except if the target file does not already exist or is different, then preserve the access and modification times of the source file. .It Fl S -Safe copy. -Normally, +Flush each file to disk after copying. +This has a non-negligible impact on performance, but reduces the risk +of being left with a partial file if the system crashes or loses power +shortly after .Nm -unlinks an existing target before installing the new file. -With the +runs. +.Pp +Historically, .Fl S -flag a temporary file is used and then renamed to be -the target. -The reason this is safer is that if the copy or -rename fails, the existing target is left untouched. +also enabled the use of temporary files to ensure atomicity when +replacing an existing target. +Temporary files are no longer optional. .It Fl s .Nm exec's the command @@ -300,15 +302,7 @@ .Sh FILES .Bl -tag -width "INS@XXXXXX" -compact .It Pa INS@XXXXXX -If either -.Fl S -option is specified, or the -.Fl C -or -.Fl p -option is used in conjunction with the -.Fl s -option, temporary files named +Temporary files named .Pa INS@XXXXXX , where .Pa XXXXXX diff --git a/usr.bin/xinstall/xinstall.c b/usr.bin/xinstall/xinstall.c --- a/usr.bin/xinstall/xinstall.c +++ b/usr.bin/xinstall/xinstall.c @@ -144,7 +144,6 @@ static int compare(int, const char *, size_t, int, const char *, size_t, char **); static char *copy(int, const char *, int, const char *, off_t); -static int create_newfile(const char *, int, struct stat *); static int create_tempfile(const char *, char *, size_t); static char *quiet_mktemp(char *template); static char *digest_file(const char *); @@ -328,10 +327,6 @@ } } - /* need to make a temp copy so we can compare stripped version */ - if (docompare && dostrip) - safecopy = 1; - /* get group and owner id's */ if (group != NULL && !dounpriv) { if (gid_from_group(group, &gid) == -1) { @@ -572,7 +567,7 @@ char tmpl[MAXPATHLEN]; int ret; - if (safecopy && target_sb != NULL) { + if (target_sb != NULL) { (void)snprintf(tmpl, sizeof(tmpl), "%s.inst.XXXXXX", to_name); /* This usage is safe. */ if (quiet_mktemp(tmpl) == NULL) @@ -619,7 +614,7 @@ { char tmpl[MAXPATHLEN]; - if (safecopy && target_sb != NULL) { + if (target_sb != NULL) { (void)snprintf(tmpl, sizeof(tmpl), "%s.inst.XXXXXX", to_name); /* This usage is safe. */ if (quiet_mktemp(tmpl) == NULL) @@ -808,7 +803,7 @@ struct stat from_sb, temp_sb, to_sb; struct timespec tsb[2]; int devnull, files_match, from_fd, serrno, stripped, target; - int tempcopy, temp_fd, to_fd; + int temp_fd, to_fd; char backup[MAXPATHLEN], *p, pathbuf[MAXPATHLEN], tempfile[MAXPATHLEN]; char *digestresult; @@ -843,16 +838,6 @@ target = (lstat(to_name, &to_sb) == 0); if (dolink) { - if (target && !safecopy) { - if (to_sb.st_mode & S_IFDIR && rmdir(to_name) == -1) - err(EX_OSERR, "%s", to_name); -#if HAVE_STRUCT_STAT_ST_FLAGS - if (to_sb.st_flags & NOCHANGEBITS) - (void)chflags(to_name, - to_sb.st_flags & ~NOCHANGEBITS); -#endif - unlink(to_name); - } makelink(from_name, to_name, target ? &to_sb : NULL); return; } @@ -863,9 +848,6 @@ return; } - /* Only copy safe if the target exists. */ - tempcopy = safecopy && target; - if (!devnull && (from_fd = open(from_name, O_RDONLY, 0)) < 0) err(EX_OSERR, "%s", from_name); @@ -886,40 +868,32 @@ } if (!files_match) { - if (tempcopy) { - to_fd = create_tempfile(to_name, tempfile, - sizeof(tempfile)); - if (to_fd < 0) - err(EX_OSERR, "%s", tempfile); - } else { - if ((to_fd = create_newfile(to_name, target, - &to_sb)) < 0) - err(EX_OSERR, "%s", to_name); - if (verbose) - (void)printf("install: %s -> %s\n", - from_name, to_name); - } + to_fd = create_tempfile(to_name, tempfile, + sizeof(tempfile)); + if (to_fd < 0) + err(EX_OSERR, "%s", tempfile); if (!devnull) { - if (dostrip) - stripped = strip(tempcopy ? tempfile : to_name, - to_fd, from_name, &digestresult); - if (!stripped) - digestresult = copy(from_fd, from_name, to_fd, - tempcopy ? tempfile : to_name, from_sb.st_size); + if (dostrip) { + stripped = strip(tempfile, to_fd, from_name, + &digestresult); + } + if (!stripped) { + digestresult = copy(from_fd, from_name, to_fd, + tempfile, from_sb.st_size); + } } } if (dostrip) { if (!stripped) - (void)strip(tempcopy ? tempfile : to_name, to_fd, - NULL, &digestresult); + (void)strip(tempfile, to_fd, NULL, &digestresult); /* * Re-open our fd on the target, in case * we did not strip in-place. */ close(to_fd); - to_fd = open(tempcopy ? tempfile : to_name, O_RDONLY, 0); + to_fd = open(tempfile, O_RDONLY, 0); if (to_fd < 0) err(EX_OSERR, "stripping %s", to_name); } @@ -963,16 +937,16 @@ digestresult = digest_file(tempfile); /* - * Move the new file into place if doing a safe copy - * and the files are different (or just not compared). + * Move the new file into place if the files are different (or + * just not compared). */ - if (tempcopy && !files_match) { + if (!files_match) { #if HAVE_STRUCT_STAT_ST_FLAGS /* Try to turn off the immutable bits. */ if (to_sb.st_flags & NOCHANGEBITS) (void)chflags(to_name, to_sb.st_flags & ~NOCHANGEBITS); #endif - if (dobackup) { + if (target && dobackup) { if ((size_t)snprintf(backup, MAXPATHLEN, "%s%s", to_name, suffix) != strlen(to_name) + strlen(suffix)) { unlink(tempfile); @@ -1222,65 +1196,6 @@ return (mkstemp(temp)); } -/* - * create_newfile -- - * create a new file, overwriting an existing one if necessary - */ -static int -create_newfile(const char *path, int target, struct stat *sbp) -{ - char backup[MAXPATHLEN]; - int saved_errno = 0; - int newfd; - - if (target) { - /* - * Unlink now... avoid ETXTBSY errors later. Try to turn - * off the append/immutable bits -- if we fail, go ahead, - * it might work. - */ -#if HAVE_STRUCT_STAT_ST_FLAGS - if (sbp->st_flags & NOCHANGEBITS) - (void)chflags(path, sbp->st_flags & ~NOCHANGEBITS); -#endif - - if (dobackup) { - if ((size_t)snprintf(backup, MAXPATHLEN, "%s%s", - path, suffix) != strlen(path) + strlen(suffix)) { - saved_errno = errno; -#if HAVE_STRUCT_STAT_ST_FLAGS - if (sbp->st_flags & NOCHANGEBITS) - (void)chflags(path, sbp->st_flags); -#endif - errno = saved_errno; - errx(EX_OSERR, "%s: backup filename too long", - path); - } - (void)snprintf(backup, MAXPATHLEN, "%s%s", - path, suffix); - if (verbose) - (void)printf("install: %s -> %s\n", - path, backup); - if (rename(path, backup) < 0) { - saved_errno = errno; -#if HAVE_STRUCT_STAT_ST_FLAGS - if (sbp->st_flags & NOCHANGEBITS) - (void)chflags(path, sbp->st_flags); -#endif - errno = saved_errno; - err(EX_OSERR, "rename: %s to %s", path, backup); - } - } else - if (unlink(path) < 0) - saved_errno = errno; - } - - newfd = open(path, O_CREAT | O_RDWR | O_TRUNC, S_IRUSR | S_IWUSR); - if (newfd < 0 && saved_errno != 0) - errno = saved_errno; - return newfd; -} - /* * copy -- * copy from one file to another