"Johannes Schindelin via GitGitGadget" <gitgitgad...@gmail.com>
writes:

> +
> +#ifndef GIT_WINDOWS_NATIVE
> +int lock_or_unlock_fd_for_appending(int fd, int lock_it)
> +{
> +     struct flock flock;
> +
> +     flock.l_type = lock_it ? F_WRLCK : F_UNLCK;
> +
> +     /* (un-)lock the whole file */
> +     flock.l_whence = SEEK_SET;
> +     flock.l_start = 0;
> +     flock.l_len = 0;
> +
> +     return fcntl(fd, F_SETLKW, &flock);
> +}
> +#endif

I think people already told you that this is not needed on systems
with properly working O_APPEND [*1*]

        Side note #1: and with network filesystems where O_APPEND
        may not work reliably, fcntl based range locking would not
        work either, so having this would not help.

I saw other Johannes and other Jeff peeking into fixing O_APPEND;
I do not know how well that effort goes, but it would be preferrable
if we can successfully go that route.  

As I said in my review of the first patch in v1 series, I am not
fundamentally opposed to a few "lock here to work around lack of
O_APPEND" and "unlock here for the same reason" calls to limited
codepaths as a workaround, as the damage is limited (that is why I
earlier looked at our use of O_APPEND), but that would be the last
resort if O_APPEND cannot be made to work reliably on Windows.

But even if we end up doing so, on systems with POSIX O_APPEND
working, I think that function should be

    #define lock_or_unlock_for_appending(fd, lock) 0 /* nothing to do */



Reply via email to