Hi, 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). 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... Christopher -- http://gmerlin.de OpenPGP: http://gmerlin.de/christopher.pub CB07 DA40 B0B6 571D 35E2 0DEF 87E2 92A7 13E5 DEE1 |
> 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.1 28 Jan 2019 18:58:42 -0000 1.40 +++ bin/cp//cp.1 1 Nov 2019 19:41:26 -0000 @@ -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 -0000 1.47 +++ bin/cp//utils.c 1 Nov 2019 19:25:02 -0000 @@ -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)); |
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? |
In reply to this post by Quentin Rameau-2
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.1 28 Jan 2019 18:58:42 -0000 1.40 >+++ bin/cp//cp.1 1 Nov 2019 19:41:26 -0000 >@@ -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 -0000 1.47 >+++ bin/cp//utils.c 1 Nov 2019 19:25:02 -0000 >@@ -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 |
> 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! |
Free forum by Nabble | Edit this page |