Re: cp -i might violate POSIX

[Timo Buhrmester <fstd.lkml%gmail.com@localhost>]

On Sat, Mar 05, 2016 at 03:03:36AM +0100, Dmitrij D. Czarkoff wrote:
> So the goal of the diff is to replace something like
> 
>   $ for src in ${files}; do [ -r "$src" ] && cp ${src} dst/; done
> 
> with
> 
>   $ yes n | cp -i ${files} dst/
> 
> ?
Or alternatively, to state in the manual that -i will be ignored
if stdin is not a tty, ideally with some rationale on why we deviate
from POSIX there.


> Well, I guess you can save some performance this way, but it is just
> too ugly to make sense.
I'd argue that
>   $ yes n | cp -i ${files} dst/
even if ugly, is more straightforward a solution to arrive at from
the manual alone than
>   $ for src in ${files}; do [ -r "$src" ] && cp ${src} dst/; done
unless one is already familiar with the (undocumented) twist.


> I'd rather add "-n" flag.
I agree that -n would be nice (below a FreeBSD-inspired patch),
but I think -i should be fixed (or the behavior documented) regardless.


----8<-----------------------------------------------------------------

--- bin/cp/cp.1.orig
+++ bin/cp/cp.1
@@ -44,7 +44,7 @@
 .Fl R
 .Op Fl H | Fl L | Fl P
 .Oc
-.Op Fl f | i
+.Op Fl f | i | n
 .Op Fl alNpv
 .Ar source_file target_file
 .Nm cp
@@ -52,7 +52,7 @@
 .Fl R
 .Op Fl H | Fl L | Fl P
 .Oc
-.Op Fl f | i
+.Op Fl f | i | n
 .Op Fl alNpv
 .Ar source_file ... target_directory
 .Sh DESCRIPTION
@@ -82,11 +82,14 @@ Same as
 For each existing destination pathname, attempt to overwrite it.
 If permissions do not allow copy to succeed, remove it and create a new
 file, without prompting for confirmation.
+.br
 (The
-.Fl i
-option is ignored if the
 .Fl f
-option is specified.)
+option overrides any previous
+.Fl i
+or
+.Fl n
+options.)
 .It Fl H
 If the
 .Fl R
@@ -100,6 +103,24 @@ that would overwrite an existing file.
 If the response from the standard input begins with the character
 .Sq Li y ,
 the file copy is attempted.
+.br
+(The
+.Fl i
+option overrides any previous
+.Fl f
+or
+.Fl n
+options.)
+.It Fl n
+Do not overwrite existing files.
+.br
+(The
+.Fl n
+option overrides any previous
+.Fl i
+or
+.Fl f
+options.)
 .It Fl L
 If the
 .Fl R
--- bin/cp/cp.c.orig
+++ bin/cp/cp.c
@@ -87,7 +87,7 @@ static char empty[] = "";
 PATH_T to = { .p_end = to.p_path, .target_end = empty  };
 
 uid_t myuid;
-int Hflag, Lflag, Rflag, Pflag, fflag, iflag, lflag, pflag, rflag, vflag, Nflag;
+int Hflag, Lflag, Rflag, Pflag, fflag, iflag, lflag, nflag, pflag, rflag, vflag, Nflag;
 mode_t myumask;
 sig_atomic_t pinfo;
 
@@ -114,7 +114,7 @@ main(int argc, char *argv[])
 	(void)setlocale(LC_ALL, "");
 
 	Hflag = Lflag = Pflag = Rflag = 0;
-	while ((ch = getopt(argc, argv, "HLNPRfailprv")) != -1) 
+	while ((ch = getopt(argc, argv, "HLNPRfailnprv")) != -1)
 		switch (ch) {
 		case 'H':
 			Hflag = 1;
@@ -142,11 +142,15 @@ main(int argc, char *argv[])
 			break;
 		case 'f':
 			fflag = 1;
-			iflag = 0;
+			nflag = iflag = 0;
 			break;
 		case 'i':
 			iflag = isatty(fileno(stdin));
-			fflag = 0;
+			fflag = nflag = 0;
+			break;
+		case 'n':
+			nflag = 1;
+			fflag = iflag = 0;
 			break;
 		case 'l':
 			lflag = 1;
--- bin/cp/extern.h.orig
+++ bin/cp/extern.h
@@ -42,7 +42,7 @@ typedef struct {
 
 extern PATH_T to;
 extern uid_t myuid;
-extern int Rflag, rflag, Hflag, Lflag, Pflag, fflag, iflag, lflag, pflag, Nflag;
+extern int Rflag, rflag, Hflag, Lflag, Pflag, fflag, iflag, lflag, nflag, pflag, Nflag;
 extern mode_t myumask;
 extern sig_atomic_t pinfo;
 
--- bin/cp/utils.c.orig
+++ bin/cp/utils.c
@@ -120,7 +120,10 @@ copy_file(FTSENT *entp, int dne)
 		struct stat sb;
 		int sval;
 
-		if (iflag) {
+		if (nflag) {
+			(void)close(from_fd);
+			return (0);
+		} else if (iflag) {
 			(void)fprintf(stderr, "overwrite %s? ", to.p_path);
 			checkch = ch = getchar();
 			while (ch != '\n' && ch != EOF)
@@ -412,8 +415,8 @@ void
 usage(void)
 {
 	(void)fprintf(stderr,
-	    "usage: %s [-R [-H | -L | -P]] [-f | -i] [-alNpv] src target\n"
-	    "       %s [-R [-H | -L | -P]] [-f | -i] [-alNpv] src1 ... srcN directory\n",
+	    "usage: %s [-R [-H | -L | -P]] [-f | -i | -n] [-alNpv] src target\n"
+	    "       %s [-R [-H | -L | -P]] [-f | -i | -n] [-alNpv] src1 ... srcN directory\n",
 	    getprogname(), getprogname());
 	exit(1);
 	/* NOTREACHED */