On Mon, Jan 07 2019 20:13:09 +0200, Lauri Tirkkonen wrote:
> @@ -128,9 +127,6 @@ main(int argc, char *argv[])
> case 'p':
> docompare = dopreserve = 1;
> break;
> - case 'S':
> - safecopy = 1;
> - break;
> case 's':
> dostrip = 1;
> break;
I realized -S is used in many places throughout src (including in
xinstall/Makefile itself, and in bsd.lib.mk, bsd.prog.mk), so -S should
probably be accepted for compatibility until those -S's can be removed.
diff revised as such.
diff --git a/usr.bin/xinstall/install.1 b/usr.bin/xinstall/install.1
index fd5db0abc37..f7d8707ce6f 100644
--- a/usr.bin/xinstall/install.1
+++ b/usr.bin/xinstall/install.1
@@ -38,7 +38,7 @@
.Nd install binaries
.Sh SYNOPSIS
.Nm install
-.Op Fl bCcDdFpSs
+.Op Fl bCcDdFps
.Op Fl B Ar suffix
.Op Fl f Ar flags
.Op Fl g Ar group
@@ -139,17 +139,6 @@ Copy the file, as if the
(compare and copy) option is specified,
except if the target file doesn't already exist or is different,
then preserve the modification time of the file.
-.It Fl S
-Safe copy.
-Normally,
-.Nm
-unlinks an existing target before installing the new file.
-With the
-.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.
.It Fl s
.Nm
exec's the command
@@ -186,18 +175,8 @@ Default is
.Sh FILES
.Bl -tag -width INS@XXXXXXXXXX -compact
.It Pa INS@XXXXXXXXXX
-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 INS@XXXXXXXXXX,
-where XXXXXXXXXX is decided by
-.Xr mkstemp 3 ,
-are created in the target directory.
+Temporary files created in the target directory by
+.Xr mkstemp 3 .
.El
.Sh EXIT STATUS
.Ex -std install
diff --git a/usr.bin/xinstall/xinstall.c b/usr.bin/xinstall/xinstall.c
index c08d82eeed4..be7311b354a 100644
--- a/usr.bin/xinstall/xinstall.c
+++ b/usr.bin/xinstall/xinstall.c
@@ -60,7 +60,7 @@
#define NOCHANGEBITS (UF_IMMUTABLE | UF_APPEND | SF_IMMUTABLE | SF_APPEND)
#define BACKUP_SUFFIX ".old"
-int dobackup, docompare, dodest, dodir, dopreserve, dostrip, safecopy;
+int dobackup, docompare, dodest, dodir, dopreserve, dostrip;
int mode = S_IRWXU|S_IRGRP|S_IXGRP|S_IROTH|S_IXOTH;
char pathbuf[PATH_MAX], tempfile[PATH_MAX];
char *suffix = BACKUP_SUFFIX;
@@ -73,7 +73,6 @@ void install(char *, char *, u_long, u_int);
void install_dir(char *, int);
void strip(char *);
void usage(void);
-int create_newfile(char *, struct stat *);
int create_tempfile(char *, char *, size_t);
int file_write(int, char *, size_t, int *, int *, int);
void file_flush(int, int);
@@ -129,7 +128,6 @@ main(int argc, char *argv[])
docompare = dopreserve = 1;
break;
case 'S':
- safecopy = 1;
break;
case 's':
dostrip = 1;
@@ -148,17 +146,13 @@ main(int argc, char *argv[])
argv += optind;
/* some options make no sense when creating directories */
- if ((safecopy || docompare || dostrip) && dodir)
+ if ((docompare || dostrip) && dodir)
usage();
/* must have at least two arguments, except when creating directories */
if (argc < 2 && !dodir)
usage();
- /* 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 && gid_from_group(group, &gid) == -1) {
gid = strtonum(group, 0, GID_MAX, &errstr);
@@ -265,54 +259,30 @@ install(char *from_name, char *to_name, u_long fset,
u_int flags)
err(1, "%s", from_name);
}
- if (safecopy) {
- to_fd = create_tempfile(to_name, tempfile, sizeof(tempfile));
- if (to_fd < 0)
- err(1, "%s", tempfile);
- } else if (docompare && !dostrip) {
- if ((to_fd = open(to_name, O_RDONLY, 0)) < 0)
- err(1, "%s", to_name);
- } else {
- if ((to_fd = create_newfile(to_name, &to_sb)) < 0)
- err(1, "%s", to_name);
- }
+ to_fd = create_tempfile(to_name, tempfile, sizeof(tempfile));
+ if (to_fd < 0)
+ err(1, "%s", tempfile);
- if (!devnull) {
- if (docompare && !safecopy) {
- files_match = !(compare(from_fd, from_name,
- from_sb.st_size, to_fd,
- to_name, to_sb.st_size));
-
- /* Truncate "to" file for copy unless we match */
- if (!files_match) {
- (void)close(to_fd);
- if ((to_fd = create_newfile(to_name, &to_sb)) <
0)
- err(1, "%s", to_name);
- }
- }
- if (!files_match)
- copy(from_fd, from_name, to_fd,
- safecopy ? tempfile : to_name, from_sb.st_size,
- ((off_t)from_sb.st_blocks * S_BLKSIZE <
from_sb.st_size));
- }
+ if (!devnull)
+ copy(from_fd, from_name, to_fd, tempfile, from_sb.st_size,
+ ((off_t)from_sb.st_blocks * S_BLKSIZE < from_sb.st_size));
if (dostrip) {
- strip(safecopy ? tempfile : to_name);
+ strip(tempfile);
/*
* Re-open our fd on the target, in case we used a strip
* that does not work in-place -- like gnu binutils strip.
*/
close(to_fd);
- if ((to_fd = open(safecopy ? tempfile : to_name, O_RDONLY,
- 0)) < 0)
+ if ((to_fd = open(tempfile, O_RDONLY, 0)) < 0)
err(1, "stripping %s", to_name);
}
/*
* Compare the (possibly stripped) temp file to the target.
*/
- if (safecopy && docompare) {
+ if (docompare) {
int temp_fd = to_fd;
struct stat temp_sb;
@@ -362,15 +332,13 @@ install(char *from_name, char *to_name, u_long fset,
u_int flags)
if ((gid != (gid_t)-1 || uid != (uid_t)-1) &&
fchown(to_fd, uid, gid)) {
serrno = errno;
- (void)unlink(safecopy ? tempfile : to_name);
- errx(1, "%s: chown/chgrp: %s",
- safecopy ? tempfile : to_name, strerror(serrno));
+ (void)unlink(tempfile);
+ errx(1, "%s: chown/chgrp: %s", tempfile, strerror(serrno));
}
if (fchmod(to_fd, mode)) {
serrno = errno;
- (void)unlink(safecopy ? tempfile : to_name);
- errx(1, "%s: chmod: %s", safecopy ? tempfile : to_name,
- strerror(serrno));
+ (void)unlink(tempfile);
+ errx(1, "%s: chmod: %s", tempfile, strerror(serrno));
}
/*
@@ -380,8 +348,7 @@ install(char *from_name, char *to_name, u_long fset, u_int
flags)
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",
- safecopy ? tempfile :to_name, strerror(errno));
+ warnx("%s: chflags: %s", tempfile, strerror(errno));
}
if (flags & USEFSYNC)
@@ -391,10 +358,10 @@ install(char *from_name, char *to_name, u_long fset,
u_int flags)
(void)close(from_fd);
/*
- * 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 (safecopy && !files_match) {
+ if (!files_match) {
/* Try to turn off the immutable bits. */
if (to_sb.st_flags & (NOCHANGEBITS))
(void)chflags(to_name, to_sb.st_flags &
~(NOCHANGEBITS));
@@ -632,7 +599,7 @@ void
usage(void)
{
(void)fprintf(stderr, "\
-usage: install [-bCcDdFpSs] [-B suffix] [-f flags] [-g group] [-m mode] [-o
owner]\n source ... target ...\n");
+usage: install [-bCcDdFps] [-B suffix] [-f flags] [-g group] [-m mode] [-o
owner]\n source ... target ...\n");
exit(1);
/* NOTREACHED */
}
@@ -657,36 +624,6 @@ create_tempfile(char *path, char *temp, size_t tsize)
return(mkstemp(temp));
}
-/*
- * create_newfile --
- * create a new file, overwriting an existing one if necessary
- */
-int
-create_newfile(char *path, struct stat *sbp)
-{
- char backup[PATH_MAX];
-
- /*
- * Unlink now... avoid ETXTBSY errors later. Try and turn
- * off the append/immutable bits -- if we fail, go ahead,
- * it might work.
- */
- if (sbp->st_flags & (NOCHANGEBITS))
- (void)chflags(path, sbp->st_flags & ~(NOCHANGEBITS));
-
- if (dobackup) {
- (void)snprintf(backup, PATH_MAX, "%s%s", path, suffix);
- /* It is ok for the target file not to exist. */
- if (rename(path, backup) < 0 && errno != ENOENT)
- err(1, "rename: %s to %s (errno %d)", path, backup,
errno);
- } else {
- if (unlink(path) < 0 && errno != ENOENT)
- err(1, "%s", path);
- }
-
- return(open(path, O_CREAT | O_RDWR | O_EXCL, S_IRUSR | S_IWUSR));
-}
-
/*
* file_write()
* Write/copy a file (during copy or archive extract). This routine knows
--
Lauri Tirkkonen | lotheac @ IRCnet
