Re: [PATCH] mingw_rmdir: do not prompt for retry when non-empty

[Erik Faye-Lund <kusmabite@gmail.com>]

Sorry for a late reply.

On Tue, Dec 4, 2012 at 5:35 PM, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> Hi kusma,
>
> On Tue, 4 Dec 2012, Erik Faye-Lund wrote:
>
>> in ab1a11be ("mingw_rmdir: set errno=ENOTEMPTY when appropriate"),
>> a check was added to prevent us from retrying to delete a directory
>> that is both in use and non-empty.
>>
>> However, this logic was slightly flawed; since we didn't return
>> immediately, we end up falling out of the retry-loop, but right into
>> the prompting loop.
>>
>> Fix this by simply returning from the function instead of breaking
>> the loop.
>>
>> While we're at it, change the second break to a return as well; we
>> already know that we won't enter the prompting-loop, beacuse
>> is_file_in_use_error(GetLastError()) already evaluated to false.
>
> I usually prefer to break from the loop, to be able to add whatever
> cleanup code we might need in the future after the loop.
>
> So does this fix the problem for you?
>
> -- snipsnap --
> diff --git a/compat/mingw.c b/compat/mingw.c
> index 04af3dc..504495a 100644
> --- a/compat/mingw.c
> +++ b/compat/mingw.c
> @@ -259,7 +259,8 @@ int mingw_rmdir(const char *pathname)
>                 return -1;
>
>         while ((ret = _wrmdir(wpathname)) == -1 && tries < ARRAY_SIZE(delay)) {
> -               if (!is_file_in_use_error(GetLastError()))
> +               errno = err_win_to_posix(GetLastError());
> +               if (errno != EACCESS)
>                         break;
>                 if (!is_dir_empty(wpathname)) {
>                         errno = ENOTEMPTY;
> @@ -275,7 +276,7 @@ int mingw_rmdir(const char *pathname)
>                 Sleep(delay[tries]);
>                 tries++;
>         }
> -       while (ret == -1 && is_file_in_use_error(GetLastError()) &&
> +       while (ret == -1 && errno == EACCESS &&
>                ask_yes_no_if_possible("Deletion of directory '%s' failed. "
>                         "Should I try again?", pathname))
>                ret = _wrmdir(wpathname);

Yes, as long as you do s/EACCESS/EACCES/ first. I don't mind such a
version instead.