Re: /bin/cp: overwrite symlink with file pointed to by symlink
> Hi Quentin, Hi Cristopher, > Thank you for having a look. You're welcome! > I know I'm very late, but I still like your patch. So ok chrisz@ if you > want to commit it. It didn't raise much passion at the time, we can just hope it gets a bit more attention now!
Re: /bin/cp: overwrite symlink with file pointed to by symlink
Hi Quentin,
Thank you for having a look.
I know I'm very late, but I still like your patch. So ok chrisz@ if you
want to commit it.
Christopher
On Fri, Nov 01, 2019 at 08:46:51PM +0100, Quentin Rameau wrote:
If you're interested in adapting to POSIX, here's a proposition patch:
Index: bin/cp//cp.1
===
RCS file: /var/cvs/src/bin/cp/cp.1,v
retrieving revision 1.40
diff -u -r1.40 cp.1
--- bin/cp//cp.128 Jan 2019 18:58:42 - 1.40
+++ bin/cp//cp.11 Nov 2019 19:41:26 -
@@ -79,9 +79,8 @@
Same as
.Fl RpP .
.It Fl f
-For each existing destination pathname, remove it and
-create a new file, without prompting for confirmation,
-regardless of its permissions.
+For each existing destination pathname which cannot be opened, remove
+it and create a new file, without prompting for confirmation.
The
.Fl f
option overrides any previous
Index: bin/cp//utils.c
===
RCS file: /var/cvs/src/bin/cp/utils.c,v
retrieving revision 1.47
diff -u -r1.47 utils.c
--- bin/cp//utils.c 28 Jan 2019 18:58:42 - 1.47
+++ bin/cp//utils.c 1 Nov 2019 19:25:02 -
@@ -55,7 +55,7 @@
static char *buf;
static char *zeroes;
struct stat to_stat, *fs;
- int from_fd, rcount, rval, to_fd, wcount;
+ int from_fd, rcount, rval, to_fd, wcount, create = 1;
#ifdef VM_AND_BUFFER_CACHE_SYNCHRONIZED
char *p;
#endif
@@ -79,26 +79,24 @@
fs = entp->fts_statp;
/*
-* In -f (force) mode, we always unlink the destination first
-* if it exists. Note that -i and -f are mutually exclusive.
-*/
- if (exists && fflag)
- (void)unlink(to.p_path);
-
- /*
* If the file DNE, set the mode to be the from file, minus setuid
* bits, modified by the umask; arguably wrong, but it makes copying
* executables work right and it's been that way forever. (The
* other choice is 666 or'ed with the execute bits on the from file
* modified by the umask.)
*/
- if (exists && !fflag) {
- if (!copy_overwrite()) {
+ if (exists) {
+ /* Note that -i and -f are mutually exclusive. */
+ if (!fflag && !copy_overwrite()) {
(void)close(from_fd);
return 2;
}
to_fd = open(to.p_path, O_WRONLY | O_TRUNC, 0);
- } else
+ if (to_fd != -1 || fflag && unlink(to.p_path) == -1)
+ create = 0;
+ }
+
+ if (create)
to_fd = open(to.p_path, O_WRONLY | O_TRUNC | O_CREAT,
fs->st_mode & ~(S_ISTXT | S_ISUID | S_ISGID));
--
http://gmerlin.de
OpenPGP: http://gmerlin.de/christopher.pub
CB07 DA40 B0B6 571D 35E2 0DEF 87E2 92A7 13E5 DEE1
Re: /bin/cp: overwrite symlink with file pointed to by symlink
Hello, > > Now I had a look at our cp.c, our cp(1) and at POSIX: > > > > -f For each existing destination pathname, remove it and > > create a new file, without prompting for confirmation, regardless of > > its permissions. The -f option overrides any previous -i options. > > > > POSIX says cp must always first try to open(2)(O_TRUNC) the > > destination and only if this fails AND -f was specified should it try > > unlink(2). > > > > At least regarding this special case our cp behaves according to > > POSIX, not according to our manpage. > > > > As long as we cannot change the POSIX standard I don't know whether > > our manpage should mention this or cp.c be changed to use lstat(), > > too. Maybe this is such a special case we could just ignore it... > > If you're interested in adapting to POSIX, here's a proposition patch: Any comment on making cp more “POSIXy” there?
Re: /bin/cp: overwrite symlink with file pointed to by symlink
> Hi,
Hello Christopher,
> I admit this is a very special case, but anyway this is what I hit:
>
> $ touch blub; ln -s blub blab; ls -l; cp -f blub blab
> total 0
> lrwxr-xr-x 1 madroach wheel 4 Oct 30 17:39 blab -> blub
> -rw-r--r-- 1 madroach wheel 0 Oct 30 17:39 blub
> cp: blab and blub are identical (not copied).
This is expected behaviour, blab and blub are indeed the same file and
so copy should be skipped.
From POSIX:
“If source_file references the same file as dest_file, cp may write a
diagnostic message to standard error; it shall do nothing more with
source_file and shall go on to any remaining files.”
> Now I had a look at our cp.c, our cp(1) and at POSIX:
>
> -f For each existing destination pathname, remove it and
> create a new file, without prompting for confirmation, regardless of
> its permissions. The -f option overrides any previous -i options.
>
> POSIX says cp must always first try to open(2)(O_TRUNC) the
> destination and only if this fails AND -f was specified should it try
> unlink(2).
>
> At least regarding this special case our cp behaves according to
> POSIX, not according to our manpage.
>
> As long as we cannot change the POSIX standard I don't know whether
> our manpage should mention this or cp.c be changed to use lstat(),
> too. Maybe this is such a special case we could just ignore it...
If you're interested in adapting to POSIX, here's a proposition patch:
Index: bin/cp//cp.1
===
RCS file: /var/cvs/src/bin/cp/cp.1,v
retrieving revision 1.40
diff -u -r1.40 cp.1
--- bin/cp//cp.128 Jan 2019 18:58:42 - 1.40
+++ bin/cp//cp.11 Nov 2019 19:41:26 -
@@ -79,9 +79,8 @@
Same as
.Fl RpP .
.It Fl f
-For each existing destination pathname, remove it and
-create a new file, without prompting for confirmation,
-regardless of its permissions.
+For each existing destination pathname which cannot be opened, remove
+it and create a new file, without prompting for confirmation.
The
.Fl f
option overrides any previous
Index: bin/cp//utils.c
===
RCS file: /var/cvs/src/bin/cp/utils.c,v
retrieving revision 1.47
diff -u -r1.47 utils.c
--- bin/cp//utils.c 28 Jan 2019 18:58:42 - 1.47
+++ bin/cp//utils.c 1 Nov 2019 19:25:02 -
@@ -55,7 +55,7 @@
static char *buf;
static char *zeroes;
struct stat to_stat, *fs;
- int from_fd, rcount, rval, to_fd, wcount;
+ int from_fd, rcount, rval, to_fd, wcount, create = 1;
#ifdef VM_AND_BUFFER_CACHE_SYNCHRONIZED
char *p;
#endif
@@ -79,26 +79,24 @@
fs = entp->fts_statp;
/*
-* In -f (force) mode, we always unlink the destination first
-* if it exists. Note that -i and -f are mutually exclusive.
-*/
- if (exists && fflag)
- (void)unlink(to.p_path);
-
- /*
* If the file DNE, set the mode to be the from file, minus setuid
* bits, modified by the umask; arguably wrong, but it makes copying
* executables work right and it's been that way forever. (The
* other choice is 666 or'ed with the execute bits on the from file
* modified by the umask.)
*/
- if (exists && !fflag) {
- if (!copy_overwrite()) {
+ if (exists) {
+ /* Note that -i and -f are mutually exclusive. */
+ if (!fflag && !copy_overwrite()) {
(void)close(from_fd);
return 2;
}
to_fd = open(to.p_path, O_WRONLY | O_TRUNC, 0);
- } else
+ if (to_fd != -1 || fflag && unlink(to.p_path) == -1)
+ create = 0;
+ }
+
+ if (create)
to_fd = open(to.p_path, O_WRONLY | O_TRUNC | O_CREAT,
fs->st_mode & ~(S_ISTXT | S_ISUID | S_ISGID));
