Hi,

On Thu, Apr 17, 2025 at 09:44:31PM +0000, NRK wrote:
> A couple facts:
> 
> 1. POSIX requires a single whole line to be read (as Roberto pointed out
>    already).
> 2. POSIX doesn't define what "affirmative" response is.
>    GNU Coreutils reads the first char and discards the rest [0].
>    Busybox skips blanks, then takes a char as answer and discards the rest 
> [1].
>    Both of these are "valid" according to POSIX.
> 
> [0]: 
> https://github.com/coreutils/gnulib/blob/71288b12a5eb299e173a17245f548d5e2adb85c0/lib/yesno.c#L54-L59
> [1]: 
> https://github.com/mirror/busybox/blob/371fe9f71d445d18be28c82a2a6d82115c8af19d/libbb/ask_confirmation.c#L11-L27
> 
> Now to Roberto's proposal:
> 
>       while (isspace(ch = getchar()))
>               ;
>       
> This is wrong because isspace() will return true on newlines, thus it'll
> end up reading *multiple* lines. We must read only one. You want
> isblank() instead.

Good catch! I just wrote without testing and that was an obvious bug ^^!

> Other than that:
> 
> 1) Skipping leading blanks seems okay to me.
> 2) Rejecting any non-space trailing char is valid by POSIX but will also
>    lead to some surprising results, such as "yes" being rejected.
> 3) IMO busybox's approach seems like a good behavior: skip leading
>    blanks, ignore trailing chars.

Based in that I rewrote the patch following the busybox approach. Please,
THIBAUT AUBIN and NRK can you review it (specially THIBAUT because you are
the author of the patch and you  will get credit for it):

---8<

>From 3a21fa0f4ed5a7fc85af3177976a087ae335013d Mon Sep 17 00:00:00 2001
From: THIBAUT AUBIN <t.aubi...@ejm.org>
Date: Thu, 10 Apr 2025 17:39:46 +0000
Subject: [PATCH] cp: add -i flag

---
 README       |  2 +-
 cp.1         |  6 ++++--
 cp.c         |  6 +++++-
 fs.h         |  1 +
 libutil/cp.c | 18 ++++++++++++++++--
 5 files changed, 27 insertions(+), 6 deletions(-)

diff --git a/README b/README
index 698eae3..fd66095 100644
--- a/README
+++ b/README
@@ -56,7 +56,7 @@ The following tools are implemented:
 0=*|o cmp             .
 0#*|x cols            .
 0=*|o comm            .
-0=*|o cp              (-i)
+0=*|o cp              .
 0=*|x cron            .
 0#*|o cut             .
 0=*|o date            .
diff --git a/cp.1 b/cp.1
index 93f2bc0..eabc02e 100644
--- a/cp.1
+++ b/cp.1
@@ -1,4 +1,4 @@
-.Dd October 8, 2015
+.Dd April 15, 2025
 .Dt CP 1
 .Os sbase
 .Sh NAME
@@ -6,7 +6,7 @@
 .Nd copy files and directories
 .Sh SYNOPSIS
 .Nm
-.Op Fl afpv
+.Op Fl afipv
 .Oo
 .Fl R
 .Op Fl H | L | P
@@ -37,6 +37,8 @@ and
 If an existing
 .Ar dest
 cannot be opened, remove it and try again.
+.It Fl i
+Interactive prompt before overwrite.
 .It Fl p
 Preserve mode, timestamp and permissions.
 .It Fl v
diff --git a/cp.c b/cp.c
index 6abe02c..c6b32f5 100644
--- a/cp.c
+++ b/cp.c
@@ -7,7 +7,7 @@
 static void
 usage(void)
 {
-       eprintf("usage: %s [-afpv] [-R [-H | -L | -P]] source ... dest\n", 
argv0);
+       eprintf("usage: %s [-afipv] [-R [-H | -L | -P]] source ... dest\n", 
argv0);
 }
 
 int
@@ -16,6 +16,9 @@ main(int argc, char *argv[])
        struct stat st;
 
        ARGBEGIN {
+       case 'i':
+               cp_iflag = 1;
+               break;
        case 'a':
                cp_follow = 'P';
                cp_aflag = cp_pflag = cp_rflag = 1;
@@ -58,3 +61,4 @@ main(int argc, char *argv[])
 
        return fshut(stdout, "<stdout>") || cp_status;
 }
+
diff --git a/fs.h b/fs.h
index 00ecd3b..f8a9322 100644
--- a/fs.h
+++ b/fs.h
@@ -28,6 +28,7 @@ enum {
 
 extern int cp_aflag;
 extern int cp_fflag;
+extern int cp_iflag;
 extern int cp_pflag;
 extern int cp_rflag;
 extern int cp_vflag;
diff --git a/libutil/cp.c b/libutil/cp.c
index 23275ac..b05cc32 100644
--- a/libutil/cp.c
+++ b/libutil/cp.c
@@ -1,4 +1,5 @@
 /* See LICENSE file for copyright and license details. */
+#include <ctype.h>
 #include <dirent.h>
 #include <errno.h>
 #include <fcntl.h>
@@ -16,6 +17,7 @@
 
 int cp_aflag  = 0;
 int cp_fflag  = 0;
+int cp_iflag  = 0;
 int cp_pflag  = 0;
 int cp_rflag  = 0;
 int cp_vflag  = 0;
@@ -26,7 +28,7 @@ int
 cp(const char *s1, const char *s2, int depth)
 {
        DIR *dp;
-       int f1, f2, flags = 0;
+       int ans, c, f1, f2, flags = 0;
        struct dirent *d;
        struct stat st;
        struct timespec times[2];
@@ -42,8 +44,20 @@ cp(const char *s1, const char *s2, int depth)
                return 0;
        }
 
+       if (cp_iflag && access(s2, F_OK) == 0) {
+               weprintf("overwrite '%s'? ", s2);
+
+               while (isspace(c = getchar()) && c != '\n')
+                       ;
+               ans = c;
+               while (c != '\n' && (c = getchar()) != EOF)
+                       ;
+               if (toupper(ans) != 'Y')
+                       return 0;
+       }
+
        if (cp_vflag)
-               printf("%s -> %s\n", s1, s2);
+               printf("'%s' -> '%s'\n", s1, s2);
 
        if (S_ISLNK(st.st_mode)) {
                if ((r = readlink(s1, target, sizeof(target) - 1)) >= 0) {
-- 
2.46.1


Reply via email to