Re: [PATCH 5/5] rename_ref(): fix a mkdir()/rmdir() race
From: Jonathan Nieder <hidden>
Date: 2016-06-15 22:59:30
Michael Haggerty wrote:
refs.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)
A test or example reproduction recipe would be nice. (But I can understand not having one --- races are hard to test.) [...]
quoted hunk ↗ jump to hunk
--- a/refs.c +++ b/refs.c
[...]
quoted hunk ↗ jump to hunk
@@ -2574,6 +2575,13 @@ int rename_ref(const char *oldrefname, const char *newrefname, const char *logms } goto retry; } else { + if (errno == ENOENT && --attempts) + /* + * Perhaps somebody just pruned the empty + * directory into which we wanted to move the + * file. + */ + goto retry;
Style nit: it's easier to read a test of errno when the 'else's cascade (i.e., using 'else if' here). This patch doesn't depend on any of the others from the series. For what it's worth, with or without the following squashed in, Reviewed-by: Jonathan Nieder <redacted> Thanks.
diff --git i/refs.c w/refs.c
index 3ab1491..ea62395 100644
--- i/refs.c
+++ w/refs.c@@ -2574,14 +2574,14 @@ int rename_ref(const char *oldrefname, const char *newrefname, const char *logms goto rollback; } goto retry; + } else if (errno == ENOENT && --attempts) + /* + * Perhaps somebody just pruned the empty + * directory into which we wanted to move the + * file. + */ + goto retry; } else { - if (errno == ENOENT && --attempts) - /* - * Perhaps somebody just pruned the empty - * directory into which we wanted to move the - * file. - */ - goto retry; error("unable to move logfile "TMP_RENAMED_LOG" to logs/%s: %s", newrefname, strerror(errno)); goto rollback;