Hi kusma,
On Wed, 5 Dec 2012, Erik Faye-Lund wrote:
> Sorry for a late reply.
Yeah, sorry, my replies tend to be delayed a lot. For the record: your
reply was not at all late.
> On Tue, Dec 4, 2012 at 5:35 PM, Johannes Schindelin
> <Johannes.Schindelin@xxxxxx> wrote:
> >
> > 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.
As you probably suspected, I did not have a way to test-compile it before
sending.
The reason I was suggesting my version of the patch was to unify the error
handling: rather than relying on both errno and GetLastError() (but for
different error conditions), I would like to rely on only one: errno. That
way, they cannot contradict each other (as they did in your case).
However, I have no strong opinion on this, so please apply the version you
like better.
Ciao,
Dscho
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html