Re: [PATCH] allow disabling fsync everywhere

[Eric Sunshine <sunshine@xxxxxxxxxxxxxx>]

On Wed, Oct 27, 2021 at 8:21 PM Eric Wong <e@xxxxxxxxx> wrote:
> "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@xxxxxxxxx>
> ---
> 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);
    }