Re: [PATCH] allow disabling fsync everywhere

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Oct 28, 2021, 01:21 UTC
Message-ID: <CAPig+cSTL-Rr+-aHqm3rsM=PLrz+BaDj7+nXGYKnKcEHgEiGOQ@mail.gmail.com>
In-Reply-To: <20211028002102.19384-1-e@80x24.org>

On Wed, Oct 27, 2021 at 8:21 PM Eric Wong <e@80x24.org> wrote:

Show 16 quoted lines

> "core.fsync" and the "GIT_FSYNC" environment variable now exist
> for disabling fsync() even on packfiles and other critical data.
> [...]
> Signed-off-by: Eric Wong <e@80x24.org>
> ---
> diff --git a/write-or-die.c b/write-or-die.c
> @@ -57,7 +58,9 @@ void fprintf_or_die(FILE *f, const char *fmt, ...)
>  void fsync_or_die(int fd, const char *msg)
>  {
> -       while (fsync(fd) < 0) {
> +       if (use_fsync < 0)
> +               use_fsync = git_env_bool("GIT_FSYNC", 1);
> +       while (use_fsync && fsync(fd) < 0) {
>                 if (errno != EINTR)
>                         die_errno("fsync error on '%s'", msg);
>         }

This is minor, but placing `use_fsync` in the `while` condition makes the logic harder to digest. The intent would be clearer at-a-glance if structured like this:

    if (use_fsync < 0)
        use_fsync = git_env_bool("GIT_FSYNC", 1);
    if (!use_fsync)
        return;
    while (fsync(fd) < 0) {
        if (errno != EINTR)
            die_errno("fsync error on '%s'", msg);
    }