[PATCH] Add additional error handling to safe_rename().

[Kevin J. McCarthy]

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.mutt.org/pipermail/mutt-dev/attachments/20180828/f4f88dcd/attachment.asc>