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.