Re: [PATCH v2 1/3] wrapper: handle EINTR in `git_fsync()`
- From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
- Date
- Nov 10, 2021, 14:39 UTC
- Message-ID
- <211110.864k8khy5m.gmgdl@evledraar.gmail.com>
- In-Reply-To
- <0c8e98295e91b656a89e1db53bfe02e7c7fc8432.1636544377.git.ps@pks.im>
On Wed, Nov 10 2021, Patrick Steinhardt wrote:
Show 53 quoted lines
> [[PGP Signed Part:Undecided]]
> While we already have a wrapper around `git_fsync()`, this wrapper
> doesn't handle looping around `EINTR`. Convert it to do so such that
> callers don't have to remember doing this everywhere.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
> wrapper.c | 9 ++++++++-
> write-or-die.c | 6 ++----
> 2 files changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/wrapper.c b/wrapper.c
> index ece3d2ca10..e20df4f3a6 100644
> --- a/wrapper.c
> +++ b/wrapper.c
> @@ -546,7 +546,7 @@ int xmkstemp_mode(char *filename_template, int mode)
> return fd;
> }
>
> -int git_fsync(int fd, enum fsync_action action)
> +static int git_fsync_once(int fd, enum fsync_action action)
> {
> switch (action) {
> case FSYNC_WRITEOUT_ONLY:
> @@ -591,7 +591,14 @@ int git_fsync(int fd, enum fsync_action action)
> default:
> BUG("unexpected git_fsync(%d) call", action);
> }
> +}
>
> +int git_fsync(int fd, enum fsync_action action)
> +{
> + while (git_fsync_once(fd, action) < 0)
> + if (errno != EINTR)
> + return -1;
> + return 0;
> }
>
> static int warn_if_unremovable(const char *op, const char *file, int rc)
> diff --git a/write-or-die.c b/write-or-die.c
> index cc8291d979..e61220aa2d 100644
> --- a/write-or-die.c
> +++ b/write-or-die.c
> @@ -57,10 +57,8 @@ void fprintf_or_die(FILE *f, const char *fmt, ...)
>
> void fsync_or_die(int fd, const char *msg)
> {
> - while (git_fsync(fd, FSYNC_HARDWARE_FLUSH) < 0) {
> - if (errno != EINTR)
> - die_errno("fsync error on '%s'", msg);
> - }
> + if (git_fsync(fd, FSYNC_HARDWARE_FLUSH) < 0)
> + die_errno("fsync error on '%s'", msg);Nit: While at it maybe change it to: "fsync() error syncing file '%s'"
Maybe having a xgit_fsync() convenience wrapper would make subsequent steps easier?