On 04/12/2012 04:02 PM, Paul Eggert wrote:
> I like this idea too. Some comments:
>
>> Preserve the specified attributes of the original files in the copy,
>> -but do not copy any data. See the @option{--preserve} option for
>> -controlling which attributes to copy.
>> +but do not copy any data. Data in existing destination files is not
>> +truncated. See the @option{--preserve} option for controlling which
>> +attributes to copy.
>
> The word "data" is plural. But there is no need to talk about
> truncation: the point is that the file's contents aren't changed at
> all. Something like this, maybe?
>
> Copy only the specified attributes of the source file to the destination.
> If the destination already exists, do not alter its contents.
> See the @option{--preserve} option for controlling which attributes to copy.
>
>
>> - dest_desc = open (dst_name, O_WRONLY | O_TRUNC | O_BINARY);
>> + /* We don't truncate with --attributes-only to support
>> + copying attributes to an existing file. */
>> + dest_desc = open (dst_name, O_WRONLY | O_BINARY
>> + | (x->data_copy_required ? O_TRUNC : 0));
>
> The indentation of the flags expression isn't right, and the
> comment is unnecessary clutter (the code is clear). Perhaps something
> like this instead?
>
> int open_flags =
> O_WRONLY | O_BINARY | (x->data_copy_required ? O_TRUNC : 0);
> dest_desc = open (dst_name, open_flags);
>
Cool.
thanks for the review,
Pádraig.
