Re: [PATCH] notes-merge: use O_EXCL to avoid overwriting existing files

[Jeff King <peff@peff.net>]

On Thu, Jul 07, 2016 at 10:08:30PM +0200, René Scharfe wrote:

> Use the open(2) flag O_EXCL to ensure the file doesn't already exist
> instead of (racily) calling stat(2) through file_exists().  While at it
> switch to xopen() to reduce code duplication and get more consistent
> error messages.

This is definitely an improvement, as it behaves the same except for the
TOCTOU race. But not being very familiar with the notes-merge code, I
have to wonder if this is a system of a larger design issue.

Why do we care that the file exists? Should we instead be using the
lockfile code to get exclusive access to it? That would also switch us
to doing the write-to-tempfile-and-rename dance, but that seems like it
would be a good thing. If we hit a write() error in the code now, we
leave a partially-written file in the notes worktree.

I dunno. From my cursory reading of the code, it seems like we'd never
really expect this file_exists() to trigger in the first place, so
perhaps it's not worth thinking too hard about it.

-Peff