You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@apr.apache.org by Joe Orton <jo...@manyfish.co.uk> on 2002/11/15 15:10:37 UTC

EEXIST from apr_dir_remove

On some platforms rmdir(2) fails with EEXIST rather than ENOTEMPTY when
trying to delete a non-empty directory; in fact POSIX specifies that
both EEXIST and ENOTEMPTY are allowed for this case.

The test_removeall_fail() test uses APR_STATUS_IS_ENOTEMPTY() for this
case. Is it okay to extend APR_STATUS_IS_ENOTEMPTY to return true for
EEXIST for this case (as below), or should the test be changed?

--- include/apr_errno.h	10 Nov 2002 08:35:16 -0000	1.101
+++ include/apr_errno.h	15 Nov 2002 14:02:55 -0000
@@ -1202,7 +1202,8 @@
 /** cross device link */
 #define APR_STATUS_IS_EXDEV(s)           ((s) == APR_EXDEV)
 /** Directory Not Empty */
-#define APR_STATUS_IS_ENOTEMPTY(s)       ((s) == APR_ENOTEMPTY)
+#define APR_STATUS_IS_ENOTEMPTY(s)       ((s) == APR_ENOTEMPTY \
+                                          (s) == APR_EEXIST)
 
 #endif /* !def OS2 || WIN32 */
 

Re: EEXIST from apr_dir_remove

Posted by "William A. Rowe, Jr." <wr...@apache.org>.
Perhaps what we need are some 'meta tests' that define exactly
what we are testing for.  Perhaps then Brane's earlier complaints
about the ENOTDIR/ENOMATCH and other strange irregularities
will disappear.

Can you come up with a good definition that all EEXIST cases should
consider ENOTEMPTY, or conversely?  If not, we really should define 
a third 'case'.

I don't have a good specific solution, so I'll listen to other comments
for a while :-/

Bill

At 08:10 AM 11/15/2002, Joe Orton wrote:
>On some platforms rmdir(2) fails with EEXIST rather than ENOTEMPTY when
>trying to delete a non-empty directory; in fact POSIX specifies that
>both EEXIST and ENOTEMPTY are allowed for this case.
>
>The test_removeall_fail() test uses APR_STATUS_IS_ENOTEMPTY() for this
>case. Is it okay to extend APR_STATUS_IS_ENOTEMPTY to return true for
>EEXIST for this case (as below), or should the test be changed?
>
>--- include/apr_errno.h 10 Nov 2002 08:35:16 -0000      1.101
>+++ include/apr_errno.h 15 Nov 2002 14:02:55 -0000
>@@ -1202,7 +1202,8 @@
> /** cross device link */
> #define APR_STATUS_IS_EXDEV(s)           ((s) == APR_EXDEV)
> /** Directory Not Empty */
>-#define APR_STATUS_IS_ENOTEMPTY(s)       ((s) == APR_ENOTEMPTY)
>+#define APR_STATUS_IS_ENOTEMPTY(s)       ((s) == APR_ENOTEMPTY \
>+                                          (s) == APR_EEXIST)
> 
> #endif /* !def OS2 || WIN32 */
> 


Re: EEXIST from apr_dir_remove

Posted by br...@xbc.nu.
Quoting rbb@rkbloom.net:
 
> I am torn about this.  On the one hand, a simple single test for this
> condition is useful.  However, EEXIST and ENOTEMPTY just don't sound like
> the same thing, so having one macro that checks for both doesn't make
> sense.  Does anybody have a strong opinion one way or the other?

If ENOTEMPTY can _only_ be returned by rmdir on a non-empty dir, and if rmdir
can return EEXIST _only_ if the directory isn't empty, then of course it makes
sense to combine the tests, and Joe's patch is correct. Otherwise, the
apr_dir_remove implementation should convert EEXIST to ENOTEMPTY ona
per-platform basis.

We definitely have to do something so that users don't have to check for both
EEXIST and ENOTEMPTY after trying to remove a dir; after all, APR is supposed to
hide platform differences. This situation is analogous to the APR_ENOENT vs.
APR_ENOTDIR one on Windows, which we solved by having APR_STATUS_IS_ENOTDIR
chaeck one of the same codes as APR_STATUS_IS_ENOENT.

    Brane


> 
> Ryan
> 
> On Fri, 15 Nov 2002, Joe Orton wrote:
> 
> > On some platforms rmdir(2) fails with EEXIST rather than ENOTEMPTY
> when
> > trying to delete a non-empty directory; in fact POSIX specifies that
> > both EEXIST and ENOTEMPTY are allowed for this case.
> >
> > The test_removeall_fail() test uses APR_STATUS_IS_ENOTEMPTY() for
> this
> > case. Is it okay to extend APR_STATUS_IS_ENOTEMPTY to return true
> for
> > EEXIST for this case (as below), or should the test be changed?
> >
> > --- include/apr_errno.h	10 Nov 2002 08:35:16 -0000	1.101
> > +++ include/apr_errno.h	15 Nov 2002 14:02:55 -0000
> > @@ -1202,7 +1202,8 @@
> >  /** cross device link */
> >  #define APR_STATUS_IS_EXDEV(s)           ((s) == APR_EXDEV)
> >  /** Directory Not Empty */
> > -#define APR_STATUS_IS_ENOTEMPTY(s)       ((s) == APR_ENOTEMPTY)
> > +#define APR_STATUS_IS_ENOTEMPTY(s)       ((s) == APR_ENOTEMPTY \
> > +                                          (s) == APR_EEXIST)
> >
> >  #endif /* !def OS2 || WIN32 */

Re: EEXIST from apr_dir_remove

Posted by rb...@rkbloom.net.
I am torn about this.  On the one hand, a simple single test for this
condition is useful.  However, EEXIST and ENOTEMPTY just don't sound like
the same thing, so having one macro that checks for both doesn't make
sense.  Does anybody have a strong opinion one way or the other?

Ryan

On Fri, 15 Nov 2002, Joe Orton wrote:

> On some platforms rmdir(2) fails with EEXIST rather than ENOTEMPTY when
> trying to delete a non-empty directory; in fact POSIX specifies that
> both EEXIST and ENOTEMPTY are allowed for this case.
>
> The test_removeall_fail() test uses APR_STATUS_IS_ENOTEMPTY() for this
> case. Is it okay to extend APR_STATUS_IS_ENOTEMPTY to return true for
> EEXIST for this case (as below), or should the test be changed?
>
> --- include/apr_errno.h	10 Nov 2002 08:35:16 -0000	1.101
> +++ include/apr_errno.h	15 Nov 2002 14:02:55 -0000
> @@ -1202,7 +1202,8 @@
>  /** cross device link */
>  #define APR_STATUS_IS_EXDEV(s)           ((s) == APR_EXDEV)
>  /** Directory Not Empty */
> -#define APR_STATUS_IS_ENOTEMPTY(s)       ((s) == APR_ENOTEMPTY)
> +#define APR_STATUS_IS_ENOTEMPTY(s)       ((s) == APR_ENOTEMPTY \
> +                                          (s) == APR_EEXIST)
>
>  #endif /* !def OS2 || WIN32 */
>
>