Re: [PATCH v2 1/3] wrapper: handle EINTR in `git_fsync()`
From: Johannes Schindelin <hidden>
Date: 2021-11-10 14:33:55
Hi Patrick, just one observation, below. On Wed, 10 Nov 2021, Patrick Steinhardt wrote:
quoted hunk ↗ jump to hunk
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; }
My immediate reaction was: why not fold `git_fsync_once()` into `git_fsync()`? And then I had a look at the function (which is sadly not in the diff context of this email, one of the occasions when I would prefer a proper UI for reviewing), and I agree that indenting the code even one level would make it harder to read. All this is to say: I agree with the approach you took here. Ciao, Dscho