[PATCH 11/11] dir.cc: Try unlink_nt first

[Corinna Vinschen]

On Jan 18 18:07, Ben wrote:
> 
> 
> On 18-01-2021 13:13, Corinna Vinschen via Cygwin-patches wrote:
> > 
> > Your code is skipping the safety checks and the has_dot_last_component()
> > check.  The latter implements a check required by POSIX.  Skipping
> > it introduces an incompatibility, see man 2 rmdir.
> > 
> 
> Yes, I missed has_dot_last_component completely.
> 
> As for the other checks:
> dir.cc: 404: fh->error ():
> * Done in unlink_nt
> dir.cc: 409: fh->exists ():
> * Done in _unlink_nt through NtOpenFile, which will return either
>   STATUS_OBJECT_NAME_NOT_FOUND or STATUS_OBJECT_PATH_NOT_FOUND,
>   both of which resolve to ENOENT
> dir.cc: 413: isdev_dev (fh->dev ()):
> * Done in unlink_nt
> fhandler_siak_file.cc: 1842:  if (!pc.isdir ())
> * Done in _unlink_nt through NtOpenFile with flags FILE_DIRECTORY_FILE
>   and FILE_NON_DIRECTORY_FILE which will return STATUS_NOT_A_DIRECTORY
>   and STATUS_FILE_IS_A_DIRECTORY respectively.
> 
> Have I missed something else?
> 
> Also, I think it's better to have isdev_dev (fh->dev ()) return EROFS,
> which is the same as unlink.

No, isdev_dev in rmdir returns ENOTEMPTY because /dev is a merge between
a virtual and a real directory.  You can write into that directory by
adding new entries, like the symlinks for stdin/stdout, etc., but
of course it's a non-empty dir.  It's a kind of stop-gap measure so
/dev doesn't get removed accidentally.

In retrospect, checking against isdev_dev is a bit unclean here.  It
would be cleaner to add a method fhandler_dev::rmdir to override
the rmdir method of the underlying fhandler_disk_file class and handle
this in the fhandler class as desired.

I pushed a matching patch.


Corinna