mingw: factor out the retry logic

In several places, Git's Windows-specific code follows the pattern where
it tries to perform an operation, and retries several times when that
operation fails, sleeping an increasing amount of time, before finally
giving up and asking the user whether to rety (after, say, closing an
editor that held a handle to a file, preventing the operation from
succeeding).

This logic is a bit hard to use, and inconsistent:
`mingw_unlink()` and `mingw_rmdir()` duplicate the code to retry,
and both of them do so incompletely. They also do not restore `errno` if the
user answers 'no'.

Introduce a `retry_ask_yes_no()` helper function that handles retry with
small delay, asking the user, and restoring `errno`.

Note that in `mingw_unlink()`, we include the `_wchmod()` call in the
retry loop (which may fail if the file is locked exclusively).

In `mingw_rmdir()`, we include special error handling in the retry loop.

Signed-off-by: Karsten Blees <blees@dcon.de>
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/compat/mingw.c b/compat/mingw.c
index c7571951dc7bd91f7022b6410cc7c0a869f35eb9..26e64c6a5a299703c32e65ddc366432540906504 100644 (file)
--- a/compat/mingw.c
+++ b/compat/mingw.c
@@ -28,8 +28,6 @@
 
 #define HCAST(type, handle) ((type)(intptr_t)handle)
 
-static const int delay[] = { 0, 1, 10, 20, 40 };
-
 void open_in_gdb(void)
 {
        static struct child_process cp = CHILD_PROCESS_INIT;
@@ -205,15 +203,12 @@ static int read_yes_no_answer(void)
        return -1;
 }
 
-static int ask_yes_no_if_possible(const char *format, ...)
+static int ask_yes_no_if_possible(const char *format, va_list args)
 {
        char question[4096];
        const char *retry_hook;
-       va_list args;
 
-       va_start(args, format);
        vsnprintf(question, sizeof(question), format, args);
-       va_end(args);
 
        retry_hook = mingw_getenv("GIT_ASK_YESNO");
        if (retry_hook) {
@@ -238,6 +233,31 @@ static int ask_yes_no_if_possible(const char *format, ...)
        }
 }
 
+static int retry_ask_yes_no(int *tries, const char *format, ...)
+{
+       static const int delay[] = { 0, 1, 10, 20, 40 };
+       va_list args;
+       int result, saved_errno = errno;
+
+       if ((*tries) < ARRAY_SIZE(delay)) {
+               /*
+                * We assume that some other process had the file open at the wrong
+                * moment and retry. In order to give the other process a higher
+                * chance to complete its operation, we give up our time slice now.
+                * If we have to retry again, we do sleep a bit.
+                */
+               Sleep(delay[*tries]);
+               (*tries)++;
+               return 1;
+       }
+
+       va_start(args, format);
+       result = ask_yes_no_if_possible(format, args);
+       va_end(args);
+       errno = saved_errno;
+       return result;
+}
+
 /* Windows only */
 enum hide_dotfiles_type {
        HIDE_DOTFILES_FALSE = 0,
@@ -298,7 +318,7 @@ static wchar_t *normalize_ntpath(wchar_t *wbuf)
 
 int mingw_unlink(const char *pathname, int handle_in_use_error)
 {
-       int ret, tries = 0;
+       int tries = 0;
        wchar_t wpathname[MAX_PATH];
        if (xutftowcs_path(wpathname, pathname) < 0)
                return -1;
@@ -306,29 +326,19 @@ int mingw_unlink(const char *pathname, int handle_in_use_error)
        if (DeleteFileW(wpathname))
                return 0;
 
-       /* read-only files cannot be removed */
-       _wchmod(wpathname, 0666);
-       while ((ret = _wunlink(wpathname)) == -1 && tries < ARRAY_SIZE(delay)) {
+       do {
+               /* read-only files cannot be removed */
+               _wchmod(wpathname, 0666);
+               if (!_wunlink(wpathname))
+                       return 0;
                if (!is_file_in_use_error(GetLastError()))
                        break;
                if (!handle_in_use_error)
-                       return ret;
+                       return -1;
 
-               /*
-                * We assume that some other process had the source or
-                * destination file open at the wrong moment and retry.
-                * In order to give the other process a higher chance to
-                * complete its operation, we give up our time slice now.
-                * If we have to retry again, we do sleep a bit.
-                */
-               Sleep(delay[tries]);
-               tries++;
-       }
-       while (ret == -1 && is_file_in_use_error(GetLastError()) &&
-              ask_yes_no_if_possible("Unlink of file '%s' failed. "
-                       "Should I try again?", pathname))
-              ret = _wunlink(wpathname);
-       return ret;
+       } while (retry_ask_yes_no(&tries, "Unlink of file '%s' failed. "
+                       "Should I try again?", pathname));
+       return -1;
 }
 
 static int is_dir_empty(const wchar_t *wpath)
@@ -355,7 +365,7 @@ static int is_dir_empty(const wchar_t *wpath)
 
 int mingw_rmdir(const char *pathname)
 {
-       int ret, tries = 0;
+       int tries = 0;
        wchar_t wpathname[MAX_PATH];
        struct stat st;
 
@@ -381,7 +391,11 @@ int mingw_rmdir(const char *pathname)
        if (xutftowcs_path(wpathname, pathname) < 0)
                return -1;
 
-       while ((ret = _wrmdir(wpathname)) == -1 && tries < ARRAY_SIZE(delay)) {
+       do {
+               if (!_wrmdir(wpathname)) {
+                       invalidate_lstat_cache();
+                       return 0;
+               }
                if (!is_file_in_use_error(GetLastError()))
                        errno = err_win_to_posix(GetLastError());
                if (errno != EACCES)
@@ -390,23 +404,9 @@ int mingw_rmdir(const char *pathname)
                        errno = ENOTEMPTY;
                        break;
                }
-               /*
-                * We assume that some other process had the source or
-                * destination file open at the wrong moment and retry.
-                * In order to give the other process a higher chance to
-                * complete its operation, we give up our time slice now.
-                * If we have to retry again, we do sleep a bit.
-                */
-               Sleep(delay[tries]);
-               tries++;
-       }
-       while (ret == -1 && errno == EACCES && is_file_in_use_error(GetLastError()) &&
-              ask_yes_no_if_possible("Deletion of directory '%s' failed. "
-                       "Should I try again?", pathname))
-              ret = _wrmdir(wpathname);
-       if (!ret)
-               invalidate_lstat_cache();
-       return ret;
+       } while (retry_ask_yes_no(&tries, "Deletion of directory '%s' failed. "
+                       "Should I try again?", pathname));
+       return -1;
 }
 
 static inline int needs_hiding(const char *path)
@@ -2384,20 +2384,8 @@ repeat:
                        SetFileAttributesW(wpnew, attrs);
                }
        }
-       if (tries < ARRAY_SIZE(delay) && gle == ERROR_ACCESS_DENIED) {
-               /*
-                * We assume that some other process had the source or
-                * destination file open at the wrong moment and retry.
-                * In order to give the other process a higher chance to
-                * complete its operation, we give up our time slice now.
-                * If we have to retry again, we do sleep a bit.
-                */
-               Sleep(delay[tries]);
-               tries++;
-               goto repeat;
-       }
        if (gle == ERROR_ACCESS_DENIED &&
-              ask_yes_no_if_possible("Rename from '%s' to '%s' failed. "
+              retry_ask_yes_no(&tries, "Rename from '%s' to '%s' failed. "
                       "Should I try again?", pold, pnew))
                goto repeat;