On 03/27/2012 01:58 PM, Philipp Thomas wrote: > I'd like to pass on observations from my collegue Neil Brown: > > in src/copy.c, copy_reg() is passed "bool *new_dst". > > This is 'false' if the file already exists, in which case it attempts to > open the file with O_WRONLY | O_TRUNC | O_BINARY. > If it is 'true', only then does it use O_CREAT (and others). > > Somewhere up the call chain - I'm not sure where - new_dst is set if 'stat' > on the file succeeds. The above mentioned code assumes that the file still > exists. This is racy - particularly for NFS where deletions from other > clients can take a while to appear.
True. That would result in: "cannot create regular file ...": ENOENT or if -f was specified a more confusing: "cannot remove ...": ENOENT You could patch to avoid that edge case, but handling dangling symlinks would complicate things. It's borderline whether this is warranted. Note the opposite case where a file is created between the stat() and the creat() would result in: "cannot create regular file ...": EEXIST even if -f was specified. Note on NFS < 3 O_EXCL is not supported so this condition won't be flagged at all. Note also handling of this case would need to honor the --no-clobber option(s). Again awkward for an unusual edge case. I suppose some comments would be appropriate at least. cheers, Pádraig.
