Re: [PATCH] mingw_rmdir: do not prompt for retry when non-empty
- From
Erik Faye-Lund <kusmabite@gmail.com>
- Date
- Dec 5, 2012, 15:18 UTC
- Message-ID
- <CABPQNSbcSEKApDBWWt7z67DvV6=JwTGebdk6hjgR1OppPyOQwg@mail.gmail.com>
- In-Reply-To
- <alpine.DEB.1.00.1212041728210.31987@s15462909.onlinehome-server.info>
Sorry for a late reply.
On Tue, Dec 4, 2012 at 5:35 PM, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:
Show 48 quoted lines
> 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.