Subject: bug#35497: [PATCH] Don't rewrite buffer contents after saving by rename

From: Jonathan Tomer <[email-masked]>
Date: Tue, 30 Apr 2019 14:10:32 -0700

On Tue, Apr 30, 2019 at 1:47 PM Michael Albinus <[email-masked]>
wrote:

> Jonathan Tomer <[email-masked]> writes:
>
> Hi Jonathan,
>
> > Thanks, I'll fix. What's the preferred mechanism for sending an
> > updated patch -- send an entirely new patch (relative to upstream) on
> > a new thread, or on this thread, or a delta to apply as an additional
> > commit on top of my previous patch?
>
> Please reply to these messages, keeping [email-masked] in To: or
> Cc:.
> This archives your message in the bug tracker.
>
> Usually, we appreciate a new patch relative to upstream. But in this
> case, not changing etc/NEWS, I believe it isn't necessary to send a new
> patch until there are other changes you want to show.
>

OK, will send along with the patch adding the TRAMP test shortly.

> That said, I'm happy to add a test to tramp-tests.el as well; at the
> > very least, with the mock tramp method we should see that the
> > destination file is renamed-to rather than overwritten as well.
>
> Pls do.
>
> Btw, your new test in files-tests.el uses file notifications. Possible
> of course. But wouldn't it be more robust to check the inode number of
> the involved files, and how it changes, or not? See file-attributes how
> to retrieve the inode number of a file.
>

I thought about checking that the inode number changes, but that wouldn't
have
caught this particular bug (where the file is renamed into place with the
correct contents, and then rewritten in place again); indeed, that doesn't
appear to be easily caught with any examination of the final state alone,
since what we're looking for is to prove the *absence* of any write that
fails
to change the inode number. (Perhaps we could check that the modification
time
of the file, after write, is *less* than its inode change time, proving that
there has been no ordinary write since the rename -- but in my experience,
inode timestamps are not actually more reliable than inotify, and in
particular this check is easily defeated by the mode-setting that happens
after the write is complete, requiring care to make sure that save-buffer
will
not attempt to do so.)

Best,
Jonathan