Patch for cp(1)

[Bruce Evans]

On Thu, 31 Mar 2005, Tom Rhodes wrote:

> On Thu, 31 Mar 2005 15:30:44 +1000 (EST)
> Bruce Evans <bde at 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.  :)

``seems to be'' was only my first impression.

>> %%%
>> 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.

This is as expected.  Special files have very special behaviour, depending
on the file.  For symlinks and fifo's, the behaviour is not very special,
but rarely what you want for a recursive cp.  Symlinks are just followed,
and operations on fifos may block somewhere, depending on whether other
processes have the fifo open and on other details.

> 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.

FreeBSD is no different.  open() of a fifo for reading hangs waiting for
a writer unless there is already data in the fifo.

> On our system, it ``just hangs'' in place of sleeping, until
> the process is killed.

This might be because you used truss to watch the process, and truss is
broken.  truss is broken for me -- doesn't show the hanging open until
truss and its child is killed with a SIGINT.  ktrace+kdump shows the
hanging open correctly.

>> 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.

No, it would be incompatible.

>> 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.

Because the implementation defines it to do what it always did,
so the proposed fix is breakage.

>> 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.

I think we don't need to keep it except for POSIX compatibility.

>>> Index: cp.c
>>> ===================================================================
>>> RCS file: /home/ncvs/src/bin/cp/cp.c,v
>>> retrieving revision 1.51
>>> @@ -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??
>

All over.  All tabs are corrupted to spaces, and this is done
inconsistently, giving identation errors like the 9-character indents
for the old and new version of the changed line in the above.

>>> @@ -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.

New programs just shouldn't use cp -r.  Old programs that use cp -r
shouldn't have its behaviour changed.

>>> 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?  :)

I meant that cp.1 in FreeBSD-1 doesn't mention -r at all.  The bitrot is
documenting -r in the compatibility section.  Without that, we would be
closer to removing -r completely.  Hmm, it's instructive that -r is
documented in the COMPATIBILTY 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:

No, the documented behaviour is that the copy is done incorrectly.
Giving more details would be like documenting undefined behaviour.

> ``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.''

These details are wrong.  What happens depends on the file type and/or
external influences.  E.g.:
- for disk files, the underlying disk is copied.  You had better be
   copying only a subset of /dev, or copying to a non-local file system,
   since the system is very unlikely to be able to hold [several] copies
   of its own disks in its own disk file systems.  (Note that the copies
   of the disk files will be regular files, so it is not impossible that
   they fit -- they may be sparesely allocated or otherwise compressed
   if the target file system does such things.)
- for ttys, the copy will hang in open() or read(), depending on ...
   Nevertheless, it may eventually complete, depending on whether someone
   or something provides EOF or EIO at the other end of the tty.
- ...
Files will never be ignored because of their file type, but files may
be skipped (with an error) because they cannot be accessed at all.
ENXIO for open() of special files with no driver is probably the most
common cause of this in RELENG_4, but devfs "fixed" this in -current.

> 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.''

This is not wrong, but it says less than the current FreeBSD manpage,
since it uses the weasel word "some" and doesn't mention symbolic links
or fifos (maybe it doesn't need to mention fifos because Sun's -r acts
differently on them).  I think the details are slightly more complicated
than for FreeBSD.  I think Sun uses open() in some cases; then it would
be open() and not open64() that hangs...

>>> 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.

Changing it is the same as breaking it, since its purpose is to
be (backwards) compatible.

Bruce