On Tue, Aug 28, 2018 at 03:27:51PM -0500, Derek Martin wrote: > On Sun, Aug 26, 2018 at 06:48:46PM -0700, Kevin J. McCarthy wrote: > > Here's a first try at a patch to check for the historically documented > > "link() returning -1 but succeeding" case for NFS. > > This seems fine.
Thank you for taking a look, Derek. > Though, it seems it leaves in the stat comparison if > link() succeeds, which is superfluous (could cause needless > performance hit on slow networked filesystems), and violates KISS and > "Don't write code you don't need" principles My strategy with these two patches was to change the code as little as possible. I agree the stats if link() succeeds are now 95% useless, but my thinking was they were there before and provided some kind of check that the/a target was actually there after the link. I could be persuaded to pull them inside the #if 0, but I figure they are doing as much harm as they have been for the past 20 years... :-/ > It obviously also does not attempt to address the follow symlink > behavior I mentioned, which is also fine. It may well be that > safe_rename() is never used in contexts where that's likely to matter, > so it's possible that it would be reasonable to ignore that issue. Yes, my thinking was like that. Technically I think you have a valid point, but the code apparently wasn't used in any context where the source was a symlink. It would have failed the compare_stat() in that case and gone into an infinite loop too, if it were. So, I again decided to minimize changes, keeping the use of lstat() consistent with previous behavior. -- Kevin J. McCarthy GPG Fingerprint: 8975 A9B3 3AA3 7910 385C 5308 ADEF 7684 8031 6BDA
signature.asc
Description: PGP signature
