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