Subject: Re: [PATCH] refs: sync loose refs to disk before committing them

From: Junio C Hamano <[email-masked]>
Date: Thu, 04 Nov 2021 14:24:26 -0700

Ævar Arnfjörð Bjarmason <[email-masked]> writes:

> I think it would probably be best to create a git_fsync_fd() function
> which is non-fatal and has that config/while loop, and have
> fsync_or_die() be a "..or die()" wrapper around that, then you could
> call that git_fsync_fd() here.

Adding git_fsync_fd() smells like a poor taste, as git_fsync() takes
an fd already.  How about making the current one into a static helper

	-int git_fsync(int fd, enum fsync_action action)
	+static int git_fsync_once(int fd, enum fsync_action action)
	 ...

and then hide the looping behavior behind git_fsync() proper?

        int git_fsync(int fd, enum fsync_action action)
        {
                while (git_fsync_once(fd, action) < 0)
                        if (errno != EINTR)
                                return -1;
                return 0;
        }

fsync_or_die() can be simplified by getting rid of its loop.

None of that needs to block Patrick's work, I think.  A version that
uses raw fsync() and punts on EINTR can graduate first, which makes
the situation better than the status quo, and all the ongoing work
that deal with fsync can be extended with an eye to make it also
usable to replace the fsync() call Patrick's fix adds.