Skip site navigation (1)Skip section navigation (2)
Date:      Thu, 15 May 2025 11:54:28 GMT
From:      Dag-Erling =?utf-8?Q?Sm=C3=B8rgrav?= <des@FreeBSD.org>
To:        src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org
Subject:   git: b3fe9014c321 - main - cp: Avoid prepending ./ in file-to-file case.
Message-ID:  <202505151154.54FBsSEQ076327@gitrepo.freebsd.org>

next in thread | raw e-mail | index | archive | help
The branch main has been updated by des:

URL: https://cgit.FreeBSD.org/src/commit/?id=b3fe9014c3214926e0347b891705aa2380f70f8c

commit b3fe9014c3214926e0347b891705aa2380f70f8c
Author:     Dag-Erling Smørgrav <des@FreeBSD.org>
AuthorDate: 2025-05-15 11:54:05 +0000
Commit:     Dag-Erling Smørgrav <des@FreeBSD.org>
CommitDate: 2025-05-15 11:54:05 +0000

    cp: Avoid prepending ./ in file-to-file case.
    
    * Make to.base large enough to hold a trailing separator.
    * Remove the separator from warning and error messages.
    * In the FILE_TO_FILE case, leave to.base empty.
    * In the FILE_TO_DIR and DIR_TO_DNE cases, append a separator to
      to.base once we've (optionally created and) opened it.
    * Thus, in the file-to-file case, we print an empty string followed by
      to.path, while in all other cases, to.base already contains the
      necessary separator.
    
    This fixes failures in tests that used cp and expected a specific
    error message and were surprised to see "./" pop up.
    
    Fixes:          82fc0d09e862
    Sponsored by:   Klara, Inc.
    Reviewed by:    markj
    Differential Revision:  https://reviews.freebsd.org/D50357
---
 bin/cp/cp.c     | 38 ++++++++++++++++++++++++++++----------
 bin/cp/extern.h |  2 +-
 bin/cp/utils.c  | 50 +++++++++++++++++++++++++-------------------------
 3 files changed, 54 insertions(+), 36 deletions(-)

diff --git a/bin/cp/cp.c b/bin/cp/cp.c
index c6b34198f20a..c76654f7df41 100644
--- a/bin/cp/cp.c
+++ b/bin/cp/cp.c
@@ -183,7 +183,12 @@ main(int argc, char *argv[])
 			sep--;
 		*sep = '\0';
 	}
-	if (strlcpy(to.base, target, sizeof(to.base)) >= sizeof(to.base))
+	/*
+	 * Copy target into to.base, leaving room for a possible separator
+	 * which will be appended later in the non-FILE_TO_FILE cases.
+	 */
+	if (strlcpy(to.base, target, sizeof(to.base) - 1) >=
+	    sizeof(to.base) - 1)
 		errc(1, ENAMETOOLONG, "%s", target);
 
 	/* Set end of argument list for fts(3). */
@@ -264,7 +269,7 @@ copy(char *argv[], enum op type, int fts_options, struct stat *root_stat)
 	struct stat created_root_stat, to_stat, *curr_stat;
 	FTS *ftsp;
 	FTSENT *curr;
-	char *recpath = NULL;
+	char *recpath = NULL, *sep;
 	int atflags, dne, badcp, len, rval;
 	mode_t mask, mode;
 	bool beneath = Rflag && type != FILE_TO_FILE;
@@ -280,11 +285,17 @@ copy(char *argv[], enum op type, int fts_options, struct stat *root_stat)
 	if (type == FILE_TO_FILE) {
 		to.dir = AT_FDCWD;
 		to.end = to.path + strlcpy(to.path, to.base, sizeof(to.path));
-		strlcpy(to.base, dot, sizeof(to.base));
+		to.base[0] = '\0';
 	} else if (type == FILE_TO_DIR) {
 		to.dir = open(to.base, O_DIRECTORY | O_SEARCH);
 		if (to.dir < 0)
 			err(1, "%s", to.base);
+		/*
+		 * We have previously made sure there is room for this.
+		 */
+		sep = strchr(to.base, '\0');
+		sep[0] = '/';
+		sep[1] = '\0';
 	}
 
 	if ((ftsp = fts_open(argv, fts_options, NULL)) == NULL)
@@ -370,13 +381,20 @@ copy(char *argv[], enum op type, int fts_options, struct stat *root_stat)
 					umask(~mask);
 				root_stat = &created_root_stat;
 				curr->fts_number = 1;
+				/*
+				 * We have previously made sure there is
+				 * room for this.
+				 */
+				sep = strchr(to.base, '\0');
+				sep[0] = '/';
+				sep[1] = '\0';
 			} else {
 				/* entering a directory; append its name to to.path */
 				len = snprintf(to.end, END(to.path) - to.end, "%s%s",
 				    to.end > to.path ? "/" : "", curr->fts_name);
 				if (to.end + len >= END(to.path)) {
 					*to.end = '\0';
-					warnc(ENAMETOOLONG, "%s/%s%s%s", to.base,
+					warnc(ENAMETOOLONG, "%s%s%s%s", to.base,
 					    to.path, to.end > to.path ? "/" : "",
 					    curr->fts_name);
 					fts_set(ftsp, curr, FTS_SKIP);
@@ -448,7 +466,7 @@ copy(char *argv[], enum op type, int fts_options, struct stat *root_stat)
 				const char *path = *to.path ? to.path : dot;
 				mode = curr_stat->st_mode;
 				if (fchmodat(to.dir, path, mode & mask, 0) != 0) {
-					warn("chmod: %s/%s", to.base, to.path);
+					warn("chmod: %s%s", to.base, to.path);
 					rval = 1;
 				}
 			}
@@ -474,7 +492,7 @@ copy(char *argv[], enum op type, int fts_options, struct stat *root_stat)
 			    to.end > to.path ? "/" : "", curr->fts_name);
 			if (to.end + len >= END(to.path)) {
 				*to.end = '\0';
-				warnc(ENAMETOOLONG, "%s/%s%s%s", to.base,
+				warnc(ENAMETOOLONG, "%s%s%s%s", to.base,
 				    to.path, to.end > to.path ? "/" : "",
 				    curr->fts_name);
 				badcp = rval = 1;
@@ -506,7 +524,7 @@ copy(char *argv[], enum op type, int fts_options, struct stat *root_stat)
 		if (!dne &&
 		    to_stat.st_dev == curr_stat->st_dev &&
 		    to_stat.st_ino == curr_stat->st_ino) {
-			warnx("%s/%s and %s are identical (not copied).",
+			warnx("%s%s and %s are identical (not copied).",
 			    to.base, to.path, curr->fts_path);
 			badcp = rval = 1;
 			if (S_ISDIR(curr_stat->st_mode))
@@ -558,7 +576,7 @@ copy(char *argv[], enum op type, int fts_options, struct stat *root_stat)
 				if (~mask & S_IRWXU)
 					umask(~mask & ~S_IRWXU);
 				if (mkdirat(to.dir, to.path, mode) != 0) {
-					warn("%s/%s", to.base, to.path);
+					warn("%s%s", to.base, to.path);
 					fts_set(ftsp, curr, FTS_SKIP);
 					badcp = rval = 1;
 					if (~mask & S_IRWXU)
@@ -568,7 +586,7 @@ copy(char *argv[], enum op type, int fts_options, struct stat *root_stat)
 				if (~mask & S_IRWXU)
 					umask(~mask);
 			} else if (!S_ISDIR(to_stat.st_mode)) {
-				warnc(ENOTDIR, "%s/%s", to.base, to.path);
+				warnc(ENOTDIR, "%s%s", to.base, to.path);
 				fts_set(ftsp, curr, FTS_SKIP);
 				badcp = rval = 1;
 				break;
@@ -611,7 +629,7 @@ copy(char *argv[], enum op type, int fts_options, struct stat *root_stat)
 			break;
 		}
 		if (vflag && !badcp)
-			(void)printf("%s -> %s/%s\n", curr->fts_path, to.base, to.path);
+			(void)printf("%s -> %s%s\n", curr->fts_path, to.base, to.path);
 	}
 	if (errno)
 		err(1, "fts_read");
diff --git a/bin/cp/extern.h b/bin/cp/extern.h
index 5a18f91ef13c..c0c524756980 100644
--- a/bin/cp/extern.h
+++ b/bin/cp/extern.h
@@ -31,8 +31,8 @@
 
 typedef struct {
 	int		 dir;		/* base directory handle */
+	char		 base[PATH_MAX + 1];	/* base directory path */
 	char		*end;		/* pointer to NUL at end of path */
-	char		 base[PATH_MAX];	/* base directory path */
 	char		 path[PATH_MAX];	/* target path */
 } PATH_T;
 
diff --git a/bin/cp/utils.c b/bin/cp/utils.c
index a849899af7ee..3e669b4b513d 100644
--- a/bin/cp/utils.c
+++ b/bin/cp/utils.c
@@ -143,12 +143,12 @@ copy_file(const FTSENT *entp, bool dne, bool beneath)
 	if (!dne) {
 		if (nflag) {
 			if (vflag)
-				printf("%s/%s not overwritten\n",
+				printf("%s%s not overwritten\n",
 				    to.base, to.path);
 			rval = 1;
 			goto done;
 		} else if (iflag) {
-			(void)fprintf(stderr, "overwrite %s/%s? %s",
+			(void)fprintf(stderr, "overwrite %s%s? %s",
 			    to.base, to.path, YESNO);
 			checkch = ch = getchar();
 			while (ch != '\n' && ch != EOF)
@@ -172,7 +172,7 @@ copy_file(const FTSENT *entp, bool dne, bool beneath)
 
 	if (lflag) {
 		if (linkat(AT_FDCWD, entp->fts_path, to.dir, to.path, 0) != 0) {
-			warn("%s/%s", to.base, to.path);
+			warn("%s%s", to.base, to.path);
 			rval = 1;
 		}
 		goto done;
@@ -180,7 +180,7 @@ copy_file(const FTSENT *entp, bool dne, bool beneath)
 
 	if (sflag) {
 		if (symlinkat(entp->fts_path, to.dir, to.path) != 0) {
-			warn("%s/%s", to.base, to.path);
+			warn("%s%s", to.base, to.path);
 			rval = 1;
 		}
 		goto done;
@@ -198,7 +198,7 @@ copy_file(const FTSENT *entp, bool dne, bool beneath)
 		    fs->st_mode & ~(S_ISUID | S_ISGID));
 	}
 	if (to_fd == -1) {
-		warn("%s/%s", to.base, to.path);
+		warn("%s%s", to.base, to.path);
 		rval = 1;
 		goto done;
 	}
@@ -220,7 +220,7 @@ copy_file(const FTSENT *entp, bool dne, bool beneath)
 		if (info) {
 			info = 0;
 			(void)fprintf(stderr,
-			    "%s -> %s/%s %3d%%\n",
+			    "%s -> %s%s %3d%%\n",
 			    entp->fts_path, to.base, to.path,
 			    cp_pct(wtotal, fs->st_size));
 		}
@@ -241,7 +241,7 @@ copy_file(const FTSENT *entp, bool dne, bool beneath)
 	if (pflag && preserve_fd_acls(from_fd, to_fd) != 0)
 		rval = 1;
 	if (close(to_fd)) {
-		warn("%s/%s", to.base, to.path);
+		warn("%s%s", to.base, to.path);
 		rval = 1;
 	}
 
@@ -260,7 +260,7 @@ copy_link(const FTSENT *p, bool dne, bool beneath)
 
 	if (!dne && nflag) {
 		if (vflag)
-			printf("%s/%s not overwritten\n", to.base, to.path);
+			printf("%s%s not overwritten\n", to.base, to.path);
 		return (1);
 	}
 	if ((len = readlink(p->fts_path, llink, sizeof(llink) - 1)) == -1) {
@@ -269,7 +269,7 @@ copy_link(const FTSENT *p, bool dne, bool beneath)
 	}
 	llink[len] = '\0';
 	if (!dne && unlinkat(to.dir, to.path, atflags) != 0) {
-		warn("unlink: %s/%s", to.base, to.path);
+		warn("unlink: %s%s", to.base, to.path);
 		return (1);
 	}
 	if (symlinkat(llink, to.dir, to.path) != 0) {
@@ -286,15 +286,15 @@ copy_fifo(struct stat *from_stat, bool dne, bool beneath)
 
 	if (!dne && nflag) {
 		if (vflag)
-			printf("%s/%s not overwritten\n", to.base, to.path);
+			printf("%s%s not overwritten\n", to.base, to.path);
 		return (1);
 	}
 	if (!dne && unlinkat(to.dir, to.path, atflags) != 0) {
-		warn("unlink: %s/%s", to.base, to.path);
+		warn("unlink: %s%s", to.base, to.path);
 		return (1);
 	}
 	if (mkfifoat(to.dir, to.path, from_stat->st_mode) != 0) {
-		warn("mkfifo: %s/%s", to.base, to.path);
+		warn("mkfifo: %s%s", to.base, to.path);
 		return (1);
 	}
 	return (pflag ? setfile(from_stat, -1, beneath) : 0);
@@ -307,15 +307,15 @@ copy_special(struct stat *from_stat, bool dne, bool beneath)
 
 	if (!dne && nflag) {
 		if (vflag)
-			printf("%s/%s not overwritten\n", to.base, to.path);
+			printf("%s%s not overwritten\n", to.base, to.path);
 		return (1);
 	}
 	if (!dne && unlinkat(to.dir, to.path, atflags) != 0) {
-		warn("unlink: %s/%s", to.base, to.path);
+		warn("unlink: %s%s", to.base, to.path);
 		return (1);
 	}
 	if (mknodat(to.dir, to.path, from_stat->st_mode, from_stat->st_rdev) != 0) {
-		warn("mknod: %s/%s", to.base, to.path);
+		warn("mknod: %s%s", to.base, to.path);
 		return (1);
 	}
 	return (pflag ? setfile(from_stat, -1, beneath) : 0);
@@ -341,7 +341,7 @@ setfile(struct stat *fs, int fd, bool beneath)
 	tspec[1] = fs->st_mtim;
 	if (fdval ? futimens(fd, tspec) :
 	    utimensat(to.dir, to.path, tspec, atflags)) {
-		warn("utimensat: %s/%s", to.base, to.path);
+		warn("utimensat: %s%s", to.base, to.path);
 		rval = 1;
 	}
 	if (fdval ? fstat(fd, &ts) :
@@ -362,7 +362,7 @@ setfile(struct stat *fs, int fd, bool beneath)
 		if (fdval ? fchown(fd, fs->st_uid, fs->st_gid) :
 		    fchownat(to.dir, to.path, fs->st_uid, fs->st_gid, atflags)) {
 			if (errno != EPERM) {
-				warn("chown: %s/%s", to.base, to.path);
+				warn("chown: %s%s", to.base, to.path);
 				rval = 1;
 			}
 			fs->st_mode &= ~(S_ISUID | S_ISGID);
@@ -372,7 +372,7 @@ setfile(struct stat *fs, int fd, bool beneath)
 	if (!gotstat || fs->st_mode != ts.st_mode) {
 		if (fdval ? fchmod(fd, fs->st_mode) :
 		    fchmodat(to.dir, to.path, fs->st_mode, atflags)) {
-			warn("chmod: %s/%s", to.base, to.path);
+			warn("chmod: %s%s", to.base, to.path);
 			rval = 1;
 		}
 	}
@@ -388,7 +388,7 @@ setfile(struct stat *fs, int fd, bool beneath)
 			 * that we copied, i.e., that we didn't create.)
 			 */
 			if (errno != EOPNOTSUPP || fs->st_flags != 0) {
-				warn("chflags: %s/%s", to.base, to.path);
+				warn("chflags: %s%s", to.base, to.path);
 				rval = 1;
 			}
 		}
@@ -409,7 +409,7 @@ preserve_fd_acls(int source_fd, int dest_fd)
 		acl_supported = 1;
 		acl_type = ACL_TYPE_NFS4;
 	} else if (ret < 0 && errno != EINVAL) {
-		warn("fpathconf(..., _PC_ACL_NFS4) failed for %s/%s",
+		warn("fpathconf(..., _PC_ACL_NFS4) failed for %s%s",
 		    to.base, to.path);
 		return (-1);
 	}
@@ -419,7 +419,7 @@ preserve_fd_acls(int source_fd, int dest_fd)
 			acl_supported = 1;
 			acl_type = ACL_TYPE_ACCESS;
 		} else if (ret < 0 && errno != EINVAL) {
-			warn("fpathconf(..., _PC_ACL_EXTENDED) failed for %s/%s",
+			warn("fpathconf(..., _PC_ACL_EXTENDED) failed for %s%s",
 			    to.base, to.path);
 			return (-1);
 		}
@@ -429,12 +429,12 @@ preserve_fd_acls(int source_fd, int dest_fd)
 
 	acl = acl_get_fd_np(source_fd, acl_type);
 	if (acl == NULL) {
-		warn("failed to get acl entries while setting %s/%s",
+		warn("failed to get acl entries while setting %s%s",
 		    to.base, to.path);
 		return (-1);
 	}
 	if (acl_is_trivial_np(acl, &trivial)) {
-		warn("acl_is_trivial() failed for %s/%s",
+		warn("acl_is_trivial() failed for %s%s",
 		    to.base, to.path);
 		acl_free(acl);
 		return (-1);
@@ -444,7 +444,7 @@ preserve_fd_acls(int source_fd, int dest_fd)
 		return (0);
 	}
 	if (acl_set_fd_np(dest_fd, acl, acl_type) < 0) {
-		warn("failed to set acl entries for %s/%s",
+		warn("failed to set acl entries for %s%s",
 		    to.base, to.path);
 		acl_free(acl);
 		return (-1);
@@ -465,7 +465,7 @@ preserve_dir_acls(const char *source_dir, const char *dest_dir)
 	dest_fd = (*dest_dir == '\0') ? to.dir :
 	    openat(to.dir, dest_dir, O_DIRECTORY, AT_RESOLVE_BENEATH);
 	if (dest_fd < 0) {
-		warn("%s: failed to copy ACLs to %s/%s", source_dir,
+		warn("%s: failed to copy ACLs to %s%s", source_dir,
 		    to.base, dest_dir);
 		close(source_fd);
 		return (-1);



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202505151154.54FBsSEQ076327>