Re: [PATCH 5/5] rename_ref(): fix a mkdir()/rmdir() race

[Jonathan Nieder <jrnieder@gmail.com>]

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.)

[...]
> --- a/refs.c
> +++ b/refs.c
[...]
> @@ -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 <jrnieder@gmail.com>

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;