Patrick Steinhardt <ps@xxxxxx> writes:
> We have a `fsync_component_or_die()` helper function which only syncs
> changes to disk in case the corresponding config is enabled by the user.
> This wrapper will always die on an error though, which makes it
> insufficient for new callsites we are about to add.
You can replace "which makes it ..." part with a bit more concrete
description to save suspense from the readers.
fsync_component_or_die() that dies upon an error is not useful
for callers with their own error handling or recovery logic,
like ref transaction API.
Split fsync_component() out that returns an error to help them.
> -void fsync_or_die(int fd, const char *);
> +int maybe_fsync(int fd);
> ...
> +static inline int fsync_component(enum fsync_component component, int fd)
> +{
> + if (fsync_components & component)
> + return maybe_fsync(fd);
> + return 0;
> +}
>
> static inline void fsync_component_or_die(enum fsync_component component, int fd, const char *msg)
> {
> - if (fsync_components & component)
> - fsync_or_die(fd, msg);
> + if (fsync_component(component, fd) < 0)
> + die_errno("fsync error on '%s'", msg);
> }
I think in the eventuall reroll, these "static inline" functions on
the I/O code path will become real functions in write-or-die.c but
other than that this reorganization looks sensible.
Thanks.
> diff --git a/write-or-die.c b/write-or-die.c
> index 9faa5f9f56..4a5455ce46 100644
> --- a/write-or-die.c
> +++ b/write-or-die.c
> @@ -56,19 +56,21 @@ void fprintf_or_die(FILE *f, const char *fmt, ...)
> }
> }
>
> -void fsync_or_die(int fd, const char *msg)
> +int maybe_fsync(int fd)
> {
> if (use_fsync < 0)
> use_fsync = git_env_bool("GIT_TEST_FSYNC", 1);
> if (!use_fsync)
> - return;
> + return 0;
>
> if (fsync_method == FSYNC_METHOD_WRITEOUT_ONLY &&
> git_fsync(fd, FSYNC_WRITEOUT_ONLY) >= 0)
> - return;
> + return 0;
>
> if (git_fsync(fd, FSYNC_HARDWARE_FLUSH) < 0)
> - die_errno("fsync error on '%s'", msg);
> + return -1;
> +
> + return 0;
> }
>
> void write_or_die(int fd, const void *buf, size_t count)
