Re: [PATCH] mkdir: alway check-for-existence

[Corinna Vinschen]

Hi Ben,

I'm fine with the patch, and it's short enough not to require an entry
in the CONTRIBUTORS file, but the commit msg needs some rephrasing:

On Jun  3 20:31, Ben wrote:
> When using either CreateDirectory or NtCreateFile when creating a directory
> that already exists, these functions return: ERROR_ALREADY_EXISTS

The Win32 and NT error codes don't match, so I'd prefer to drop
CreateDirectory from the text and use the NT status code names,
STATUS_OBJECT_NAME_COLLISION and STATUS_ACCESS_DENIED.

> However when using these function to create a directory (and all its
> parents)
> a normal use would be to start with mkdir(‘/c’) which translates to ‘C:\’

mkdir('/c') has no meaning by default.  The text should refer to the
default path "/cygdrive/c".

> for which both of these functions return ‘ERROR_ACCESS_DENIED’
> 
> We could call NtCreateFile with 'FILE_OPEN_IF' instead of 'FILE_CREATE' but
> since that's an internal function we better always check for existence
> ourselves.

Not sure what you're trying to say here.  In how far would using
FILE_OPEN_IF help?  AFAICS it would just erroneously return
STATUS_SUCCESS, so fhandler_disk_file::mkdir would have to check
io.Information for FILE_OPENED or something like that.  Maybe this
paragraph can just go away?


Thanks,
Corinna

-- 
Corinna Vinschen
Cygwin Maintainer


signature.asc
Description: PGP signature

[PATCH] mkdir: alway check-for-existence

[Ben]

When using either CreateDirectory or NtCreateFile when creating a directory
that already exists, these functions return: ERROR_ALREADY_EXISTS

However when using these function to create a directory (and all its 
parents)

a normal use would be to start with mkdir(‘/c’) which translates to ‘C:\’
for which both of these functions return ‘ERROR_ACCESS_DENIED’

We could call NtCreateFile with 'FILE_OPEN_IF' instead of 'FILE_CREATE' but
since that's an internal function we better always check for existence
ourselves.

Signed-off-by: Ben Wijen 
---
 winsup/cygwin/dir.cc | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/winsup/cygwin/dir.cc b/winsup/cygwin/dir.cc
index f43eae461..b757851d5 100644
--- a/winsup/cygwin/dir.cc
+++ b/winsup/cygwin/dir.cc
@@ -331,8 +331,10 @@ mkdir (const char *dir, mode_t mode)
       debug_printf ("got %d error from build_fh_name", fh->error ());
       set_errno (fh->error ());
     }
+  else if (fh->exists ())
+    set_errno (EEXIST);
   else if (has_dot_last_component (dir, true))
-    set_errno (fh->exists () ? EEXIST : ENOENT);
+    set_errno (ENOENT);
   else if (!fh->mkdir (mode))
     res = 0;
   delete fh;

--
2.21.0