Date: Thu, 31 Mar 2005 15:19:08 -0500 From: Tom Rhodes <trhodes@FreeBSD.org> To: Bruce Evans <bde@zeta.org.au> Cc: standards@FreeBSD.org Subject: Re: Patch for cp(1) Message-ID: <20050331151908.498559bd@mobile.pittgoth.com> In-Reply-To: <20050331144552.L20243@delplex.bde.org> References: <20050330181904.16519571@mobile.pittgoth.com> <20050331144552.L20243@delplex.bde.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Thu, 31 Mar 2005 15:30:44 +1000 (EST)
Bruce Evans <bde@zeta.org.au> wrote:
> On Wed, 30 Mar 2005, Tom Rhodes wrote:
>
> > Hi -standards.
> >
> > What do people think of the following patch to cp.c:
>
> It seems to be wrong. From cp(1):
``seems'' is the word I'm looking at. :)
>
> %%%
> COMPATIBILITY
> Historic versions of the cp utility had a -r option. This implementation
> supports that option, however, its use is strongly discouraged, as it
> does not correctly copy special files, symbolic links or fifo's.
> %%%
It hangs on copying certain files. i.e.: It just sits there
waiting for some kind of input. On SunOS 5.9, I get the same
behavior; however, on their platform it's because the open64()
call will hang while waiting for input from the fifo.
On our system, it ``just hangs'' in place of sleeping, until
the process is killed.
>
> Any change to the semantics of -r would break applications that depend on
> its historical behaviour. Any change that doesn't change the man page would
> be more broken.
What, it would actually copy special files now? That seems to
be a good thing.
>
> This conforms to at least POSIX.1-200x-draft7:
>
> %%%
> 10284 OB cp -r [-H | -L | -P][-fip] source_file ... target
> 10326 * If the -r option was specified, the behavior is implementation-defined.
> 10430 OB -r Copy file hierarchies. The treatment of special files is implementation-defined.
> %%%
Yes I know that -r is implementation-defined and I'm talking
about making it work right without adding a bunch of new code.
Why not just fix? Because POSIX warns that the -r option will
be obsoleted in the next revision.
>
> The correct fix is to remove the -r option. Unfortunately, POSIX broke
> this.
I agree, but we need to keep it for reasons you specify. At
least for the time being.
>
> > Index: cp.c
> > ===================================================================
> > RCS file: /home/ncvs/src/bin/cp/cp.c,v
> > retrieving revision 1.51
> > diff -u -r1.51 cp.c
> > --- cp.c 10 Jan 2005 08:39:21 -0000 1.51
> > +++ cp.c 30 Mar 2005 15:42:43 -0000
> > @@ -84,7 +84,7 @@
> > PATH_T to = { to.p_path, emptystring, "" };
> >
> > int fflag, iflag, nflag, pflag, vflag;
> > -static int Rflag, rflag;
> > +static int Rflag;
> > volatile sig_atomic_t info;
> >
> > enum op { FILE_TO_FILE, FILE_TO_DIR, DIR_TO_DNE };
> > @@ -135,7 +135,7 @@
> > pflag = 1;
> > break;
> > case 'r':
> > - rflag = 1;
> > + Rflag = 1;
> > break;
> > case 'v':
> > vflag = 1;
>
> The patch has lots of tab lossage and collateral indentation lossage...
In the space below??
>
> > @@ -151,16 +151,7 @@
> > usage();
> >
> > fts_options = FTS_NOCHDIR | FTS_PHYSICAL;
> > - if (rflag) {
> > - if (Rflag)
> > - errx(1,
> > - "the -R and -r options may not be specified together.");
> > - if (Hflag || Lflag || Pflag)
> > - errx(1,
> > - "the -H, -L, and -P options may not be specified with the -r option.");
> > - fts_options &= ~FTS_PHYSICAL;
> > - fts_options |= FTS_LOGICAL;
> > - }
>
> POSIX.1-200x-draft7 only requires -{HLP} to work with -R. Their behaviour
> with -r is not explicitly mentioned. It is strange to show them in
> the synopsis with -r when they are errors when actually used with -r.
> However, I think they should cause an error when used with -r, since
> their use with -r is not supported. -r could be made to work the same
> as -R iff it is used together with at least one pf -{HLP}, but that
> would be wrong since it would undeprecate -r.
Exactly. We can still ERR for -r.
>
> > +
>
> ... and wrong whitespace gainage.
Yes sir. :)
>
> > if (Rflag) {
> > if (Hflag)
> > fts_options |= FTS_COMFOLLOW;
> > @@ -224,12 +215,12 @@
> > * the initial mkdir().
> > */
> > if (r == -1) {
> > - if (rflag || (Rflag && (Lflag || Hflag)))
> > + if ((Rflag && (Lflag || Hflag))
> > stat(*argv, &tmp_stat);
> > else
> > lstat(*argv, &tmp_stat);
>
> This is the main (only?) place where -r gives useful behaviour that is
> significantly different from -R. With a bare -r, cp always follows
> symlinks, but with a bare -R, cp never follows symlinks. (The behaviour
> of cp -r on non-regular files is not useful.)
The only? Users can still use -RH, the -r option was removed
from usage() awhile ago. I'm just trying to make life easier
on those who still may accidently use -r on special files,
while removing some (unused?) code.
>
> > ...
>
> > So, why/what am I doing:
> >
> > My copy of SuSv3 states that -r is about to become obsolete;
>
> My copy of cp.1 says that it became obsolete in BSD before 4.4BSDLite1 :-).
> It was obsolete long before that too, since it was obolete in FreeBSD-1.
> cp.1 in FreeBSD-1 is dated July 30, 1991. The deprecation apparently
> rotted a bit between Net/2 and 4.4BSD -- cp.1 in FreeBSD doesn't mention
> -r at all.
Other than in the compatibility section? :)
>
> > The -r option fails (actually hangs) when trying to copy a fifo
> > file within a directory. It does this on both CURRENT,
> > STABLE and SunOS 5.9.
>
> This is the documented behaviour.
No, this is not the documented behavior. If this was the truth,
the documenation would say:
``The -r option has become obsolete and will hang while copying
special files. In some cases, the directory or files will
be copied, but the special files will be ignored.''
On SunOS, correct documentation would be:
``Use of the -r option on some special files will cause the
open64() system call to sleep, waiting for system input.
In other occasions, some files might not be properly copied.''
>
> > The idea was to make -r a synonym for -R, which works in all of
> > these cases.
> >
> > I plan to fix the manual page as -r is not historical, it was
> > implemenation dependent. Comments/suggestions? Yes, manual
> > page commit would done together of course.
>
> -r is historical. POSIX permits it to be implementation-defined
> to prevent breaking it.
And we won't be breaking it. We won't be bringing it back.
We're only going to remove it's functionality and let it work
as if to be -R.
--
Tom Rhodes
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20050331151908.498559bd>