Wayne Davison
2001-May-12 06:26 UTC
Erase the source file open; specify a tempfile name option
I'm curious how to go about submitting a suggestion that affects both
the original BSD version and the portable release. A few days ago I
sent off a BSD-relative patch to openssh at openssh.com. Is this the right
thing to do? I didn't hear anything back, but it's only been 3 days, so
I'm probably just being too antsy.
In the meantime, maybe someone else out there would like to check this
out. Appended is a version of my patch for the latest portable code
(relative to the CVS version). It adds two new options to scp that I
find useful:
-E Erase the source file after a successful copy.
-T file Use "file" as a temporary file that gets renamed into
each actual destination file.
I implemented this patch in order to be able to move input files from
one system to another, while also not having the files show up in the
destination directory until each one was complete (there's a program
reading the input dir that expects any file in the destination dir to
be fully written). These two new options make this easy. Yes, the
user must specify a -T option that is on the same file system as the
destination dir or the rename will fail (with an appropriate error and
without erasing the source file).
I haven't changed any documentation yet. I'd be glad to if this gets
accepted.
Enjoy,
..wayne..
---8<------8<------8<------8<---cut
here--->8------>8------>8------>8---
Index: scp.c
@@ -105,6 +105,7 @@
/* Returns width of the terminal (for progress meter calculations). */
int getttywidth(void);
+
int do_cmd(char *host, char *remuser, char *cmd, int *fdin, int *fdout, int
argc);
/* Struct for addargs */
@@ -206,9 +207,11 @@
uid_t userid;
int errs, remin, remout;
int pflag, iamremote, iamrecursive, targetshouldbedirectory;
+int eraseflag;
+char *tmpfn = "";
-#define CMDNEEDS 64
-char cmd[CMDNEEDS]; /* must hold "rcp -r -p -d\0" */
+char *cmd;
+int cmdlen;
int response(void);
void rsource(char *, struct stat *);
@@ -236,7 +239,7 @@
addargs(&args, "-oFallBackToRsh no");
fflag = tflag = 0;
- while ((ch = getopt(argc, argv, "dfprtvBCc:i:P:q46S:o:")) != -1)
+ while ((ch = getopt(argc, argv, "dfprtvBCc:i:P:q46S:o:ET:")) != -1)
switch (ch) {
/* User-visible flags. */
case '4':
@@ -270,6 +273,12 @@
case 'q':
showprogress = 0;
break;
+ case 'E':
+ eraseflag = 1;
+ break;
+ case 'T':
+ tmpfn = xstrdup(optarg);
+ break;
/* Server options. */
case 'd':
@@ -319,8 +328,11 @@
remin = remout = -1;
/* Command to be executed on remote system using "ssh". */
- (void) snprintf(cmd, sizeof cmd, "scp%s%s%s%s",
+ cmdlen = strlen(tmpfn) + 64;
+ cmd = xmalloc(cmdlen);
+ (void) snprintf(cmd, cmdlen, "scp%s%s%s%s%s%s%s",
verbose_mode ? " -v" : "",
+ eraseflag ? " -E" : "", *tmpfn ? " -T" :
"", tmpfn,
iamrecursive ? " -r" : "", pflag ? " -p" :
"",
targetshouldbedirectory ? " -d" : "");
@@ -370,7 +382,7 @@
host = strchr(argv[i], '@');
len = strlen(ssh_program) + strlen(argv[i]) +
strlen(src) + (tuser ? strlen(tuser) : 0) +
- strlen(thost) + strlen(targ) + CMDNEEDS + 32;
+ strlen(thost) + strlen(targ) + cmdlen + 32;
bp = xmalloc(len);
if (host) {
*host++ = 0;
@@ -403,7 +415,7 @@
(void) xfree(bp);
} else { /* local to remote */
if (remin == -1) {
- len = strlen(targ) + CMDNEEDS + 20;
+ len = strlen(targ) + cmdlen + 20;
bp = xmalloc(len);
(void) snprintf(bp, len, "%s -t %s", cmd, targ);
host = cleanhostname(thost);
@@ -428,7 +440,8 @@
char *bp, *host, *src, *suser;
for (i = 0; i < argc - 1; i++) {
- if (!(src = colon(argv[i]))) { /* Local to local. */
+ src = colon(argv[i]);
+ if (!src && !eraseflag && !*tmpfn) { /* Local to local w/cp
*/
len = strlen(_PATH_CP) + strlen(argv[i]) +
strlen(argv[argc - 1]) + 20;
bp = xmalloc(len);
@@ -441,23 +454,30 @@
++errs;
(void) xfree(bp);
continue;
+ }
+ if (src) {
+ *src++ = 0;
+ if (*src == 0)
+ src = ".";
+ if ((host = strchr(argv[i], '@')) == NULL) {
+ host = argv[i];
+ suser = NULL;
+ } else {
+ *host++ = 0;
+ suser = argv[i];
+ if (*suser == '\0')
+ suser = pwd->pw_name;
+ else if (!okname(suser))
+ continue;
+ }
+ host = cleanhostname(host);
}
- *src++ = 0;
- if (*src == 0)
- src = ".";
- if ((host = strchr(argv[i], '@')) == NULL) {
- host = argv[i];
+ else {
+ src = argv[i];
+ host = "localhost";
suser = NULL;
- } else {
- *host++ = 0;
- suser = argv[i];
- if (*suser == '\0')
- suser = pwd->pw_name;
- else if (!okname(suser))
- continue;
}
- host = cleanhostname(host);
- len = strlen(src) + CMDNEEDS + 20;
+ len = strlen(src) + cmdlen + 20;
bp = xmalloc(len);
(void) snprintf(bp, len, "%s -f %s", cmd, src);
if (do_cmd(host, suser, bp, &remin, &remout, argc) < 0) {
@@ -582,7 +602,10 @@
(void) atomicio(write, remout, "", 1);
else
run_err("%s: %s", name, strerror(haderr));
- (void) response();
+ if (response() == 0 && eraseflag && !haderr) {
+ if (unlink(name) < 0)
+ run_err("%s: %s", name, strerror(errno));
+ }
}
}
@@ -656,7 +679,7 @@
int amt, count, exists, first, mask, mode, ofd, omode;
off_t size;
int setimes, targisdir, wrerrno = 0;
- char ch, *cp, *np, *targ, *why, *vect[1], buf[2048];
+ char ch, *cp, *np, *targ, *dest, *why, *vect[1], buf[2048];
struct timeval tv[2];
#define atime tv[0]
@@ -770,6 +793,7 @@
np = namebuf;
} else
np = targ;
+ dest = *tmpfn? tmpfn : np;
curfile = cp;
exists = stat(np, &stb) == 0;
if (buf[0] == 'D') {
@@ -804,8 +828,8 @@
}
omode = mode;
mode |= S_IWRITE;
- if ((ofd = open(np, O_WRONLY | O_CREAT | O_TRUNC, mode)) < 0) {
-bad: run_err("%s: %s", np, strerror(errno));
+ if ((ofd = open(dest, O_WRONLY | O_CREAT | O_TRUNC, mode)) < 0) {
+bad: run_err("%s: %s", dest, strerror(errno));
continue;
}
(void) atomicio(write, remout, "", 1);
@@ -861,7 +885,7 @@
}
#if 0
if (ftruncate(ofd, size)) {
- run_err("%s: truncate: %s", np, strerror(errno));
+ run_err("%s: truncate: %s", dest, strerror(errno));
wrerr = DISPLAYED;
}
#endif
@@ -870,19 +894,19 @@
#ifdef HAVE_FCHMOD
if (fchmod(ofd, omode))
#else /* HAVE_FCHMOD */
- if (chmod(np, omode))
+ if (chmod(dest, omode))
#endif /* HAVE_FCHMOD */
run_err("%s: set mode: %s",
- np, strerror(errno));
+ dest, strerror(errno));
} else {
if (!exists && omode != mode)
#ifdef HAVE_FCHMOD
if (fchmod(ofd, omode & ~mask))
#else /* HAVE_FCHMOD */
- if (chmod(np, omode & ~mask))
+ if (chmod(dest, omode & ~mask))
#endif /* HAVE_FCHMOD */
run_err("%s: set mode: %s",
- np, strerror(errno));
+ dest, strerror(errno));
}
if (close(ofd) == -1) {
wrerr = YES;
@@ -891,15 +915,21 @@
(void) response();
if (setimes && wrerr == NO) {
setimes = 0;
- if (utimes(np, tv) < 0) {
+ if (utimes(dest, tv) < 0) {
run_err("%s: set times: %s",
- np, strerror(errno));
+ dest, strerror(errno));
wrerr = DISPLAYED;
}
}
+ if (*tmpfn && rename(tmpfn, np) < 0) {
+ wrerr = YES;
+ wrerrno = errno;
+ }
switch (wrerr) {
case YES:
- run_err("%s: %s", np, strerror(wrerrno));
+ if (*tmpfn)
+ unlink(tmpfn);
+ run_err("%s: %s", dest, strerror(wrerrno));
break;
case NO:
(void) atomicio(write, remout, "", 1);
@@ -949,8 +979,8 @@
void
usage()
{
- (void) fprintf(stderr, "usage: scp "
- "[-pqrvBC46] [-S ssh] [-P port] [-c cipher] [-i identity] f1
f2\n"
+ (void) fprintf(stderr, "usage: scp [-pqrvBCE46] "
+ "[-S ssh] [-P port] [-c cipher] [-i id] [-T tmp] f1 f2\n"
" or: scp [options] f1 ... fn directory\n");
exit(1);
}
---8<------8<------8<------8<---cut
here--->8------>8------>8------>8---
Kaelin Colclasure
2001-May-14 15:47 UTC
Erase the source file open; specify a tempfile name option
Wayne Davison wrote:> > I'm curious how to go about submitting a suggestion that affects both > the original BSD version and the portable release. A few days ago I > sent off a BSD-relative patch to openssh at openssh.com. Is this the right > thing to do? I didn't hear anything back, but it's only been 3 days, so > I'm probably just being too antsy. > > In the meantime, maybe someone else out there would like to check this > out. Appended is a version of my patch for the latest portable code > (relative to the CVS version). It adds two new options to scp that I > find useful: > > -E Erase the source file after a successful copy. > -T file Use "file" as a temporary file that gets renamed into > each actual destination file.Hmmm, might it not be better to have the -T option generate a temporary file name for each file in the copy on its own? After all, it is possible that a recursive copy might cross a mount point on the destination system. And in any case it's one less error that the user might make. -- Kaelin
Markus Friedl
2001-May-19 15:51 UTC
Erase the source file open; specify a tempfile name option
On Fri, May 11, 2001 at 11:26:13PM -0700, Wayne Davison wrote:> I'm curious how to go about submitting a suggestion that affects both > the original BSD version and the portable release. A few days ago I > sent off a BSD-relative patch to openssh at openssh.com. Is this the right > thing to do? I didn't hear anything back, but it's only been 3 days, so > I'm probably just being too antsy. > > In the meantime, maybe someone else out there would like to check this > out. Appended is a version of my patch for the latest portable code > (relative to the CVS version). It adds two new options to scp that I > find useful: > > -E Erase the source file after a successful copy. > -T file Use "file" as a temporary file that gets renamed into > each actual destination file.i'm not sure about this. should we really modify scp? i'd prefer to have a minimal diff between scp.c and rcp.c -m
Wayne Davison
2001-May-20 08:31 UTC
Erase the source file open; specify a tempfile name option
On Sun, 20 May 2001, Theo de Raadt wrote:> Actually, that is not the unix philosophy.Consider why "mv" exists when we have "cp" and "rm". Or why sort has a -u option when uniq exists. Or why grep has a -s (-q) option when we could just pipe stdout to /dev/null. Yes, part of the unix philosophy is to create modular tools and use them together. But another part is to add (sensible) options to tools that make accomplishing common tasks easier, even if we could have used several separate tools to accomplish the task in a less convenient manner. I think the key questions are: is this option sensible for this particular tool (i.e. does it solve a problem closely related to the core use of the utility and do so in a better or more efficient way than using a separate utility) and/or does the option make a common task easier. ..wayne..
Markus Friedl
2001-May-20 10:58 UTC
Erase the source file open; specify a tempfile name option
On Sun, May 20, 2001 at 01:31:43AM -0700, Wayne Davison wrote:> On Sun, 20 May 2001, Theo de Raadt wrote: > > Actually, that is not the unix philosophy. > > Consider why "mv" exists when we have "cp" and "rm".because mv a b does not copy and is very different from cp a b; rm a> Or why sort has a > -u option when uniq exists.i always considered this a bug :)> I think the key questions are: is > this option sensible for this particular tool (i.e. does it solve a > problem closely related to the core use of the utility and do so in a > better or more efficient way than using a separate utility) and/or does > the option make a common task easier.i think that both rcp and scp are very basic tools, just like cp. if you want to do more complicated tasks you can use tar over ssh and some scripting or use rsync. btw, i really like to read Rob Pike, "UNIX Style, or cat -v Considered Harmful", USENIX Summer Conference Proceedings, 1983.