Re: [PATCH] rm: add --preserve-root=all to protect mounts

[Jim Meyering]

On Mon, Jun 18, 2018 at 8:16 AM, Jim Meyering <address@hidden> wrote:
> On Sun, Jun 17, 2018 at 9:41 PM, Pádraig Brady <address@hidden> wrote:
>> On 17/06/18 21:16, Jim Meyering wrote:
>>> On Sun, Jun 17, 2018 at 8:44 PM, Pádraig Brady <address@hidden> wrote:
>>>> I'll apply the attached full patch soon.
>>>
>>> Thanks for doing all that. I like it.
>>> One nit: this says "on a separate device":
>>>
>>> +      --preserve-root[=all]  do not remove '/' (default); with 'all', 
>>> skip\n\
>>> +                              any command line argument on a separate 
>>> device\n\
>>>
>>> Perhaps it should say something like "on a separate device from its
>>> parent" ? Otherwise, one might erroneously interpret "separate" as
>>> meaning "different from the device of other command line arguments".
>>>
>>> I wondered how the code tested for this attribute, and had to read the
>>> code to find that it stats both the command-line argument, A, and
>>> "A/..", and if they have different device IDs, then A is eligible to
>>> be skipped when this new option is in effect.
>>
>> Yes the texinfo mentions the parent.
>> I'll expand --help to mention that also:
>>
>>   --preserve-root[=all]  do not remove '/' (default);
>>                           with 'all', skip any command line argument
>>                           on a separate device from its parent
>
> Thanks, clearly I missed the texinfo addition :-)
> Speaking of that, please use the active voice, not passive, i.e., change this
>
>   When @samp{all} is specified, any command line argument that
>   is on a separate file system to its parent is also skipped.
>
> to this:
>
>   When @samp{all} is specified, skip any command line argument that
>   is not on the same file system as its parent.

Actually, please also change s/skip/reject/; that is more appropriate,
since that rejection results in a nonzero exit status.