Re: [PATCH v3 5/5] mingw_rmdir: set errno=ENOTEMPTY when appropriate

From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Dec 15, 2010, 00:21 UTC
Message-ID: <alpine.DEB.1.00.1012150114010.1461@bonsai2>
In-Reply-To: <AANLkTi=12WqYn5GJpqPYqKE_2L51Em7Ek5szTZYryHd9@mail.gmail.com>

Hi,

On Tue, 14 Dec 2010, Erik Faye-Lund wrote:

Show 13 quoted lines

> On Tue, Dec 14, 2010 at 11:28 PM, Heiko Voigt <hvoigt@hvoigt.net> wrote:
> > From: Johannes Schindelin <johannes.schindelin@gmx.de>
> >
> > On Windows, EACCES overrules ENOTEMPTY when calling rmdir(). But if the
> > directory is busy, we only want to retry deleting the directory if it
> > is empty, so test specifically for that case and set ENOTEMPTY rather
> > than EACCES.
> >
> 
> Hmm... According to MSDN, rmdir(*) should already handle ENOTEMPTY. 
> Isn't the problem rather the structure of that loop? Shouldn't it be 
> sufficient to do something like this (note: untested, but the concept 
> should work, no)?

This is how the patch looks in current 4msysgit.git's devel branch:

-- snip --
 #undef rmdir
 int mingw_rmdir(const char *pathname)
 {
-    int ret, tries = 0;
+       int ret, tries = 0;
 
        while ((ret = rmdir(pathname)) == -1 && tries < ARRAY_SIZE(delay)) 
{
                if (errno != EACCES)
                        break;
+               if (!is_dir_empty(pathname)) {
+                       errno = ENOTEMPTY;
+                       break;
+               }
                /*
                 * We assume that some other process had the source or
                 * destination file open at the wrong moment and retry.
-- snap --

Of course, with so much water running down the Elbe between me writing that patch and me answering you, I cannot really say whether rmdir() set errno to ENOTEMPTY.

But as the patch looked the same when I wrote it originally (you can see it in the history, since I introduced rebasing merges prior to making this patch) I would assume that in my tests, errno was set to EACCESS rather than ENOTEMPTY.

However, since I am distrusted on the Git mailing list it would be good if you tried your version and verified whether what I say is true.

Thanks, Dscho