Signed-off-by: Daniel Lehman dlehman25@gmail.com --- v3: use fdopendir(dup()) v2: use getdents
needed by std::filesystem::remove_all, which removes files with: - CreateFileW - SetFileInformationByHandle(FileDispositionInfoEx) // currently unimplemented - SetFileInformationByHandle(FileDispositionInfo) - DeleteFile = TRUE - CloseHandle
--- dlls/ntdll/tests/file.c | 3 --- server/fd.c | 51 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 3 deletions(-)
diff --git a/dlls/ntdll/tests/file.c b/dlls/ntdll/tests/file.c index 31c18454f0..1d0682a633 100644 --- a/dlls/ntdll/tests/file.c +++ b/dlls/ntdll/tests/file.c @@ -3185,17 +3185,14 @@ todo_wine CloseHandle( handle2 ); fdi.DoDeleteFile = TRUE; res = pNtSetInformationFile( handle, &io, &fdi, sizeof fdi, FileDispositionInformation ); - todo_wine ok( res == STATUS_DIRECTORY_NOT_EMPTY, "unexpected FileDispositionInformation result (expected STATUS_DIRECTORY_NOT_EMPTY, got %x)\n", res ); fileDeleted = DeleteFileA( buffer ); ok( fileDeleted, "File should have been deleted\n" ); buffer[dirpos] = '\0'; CloseHandle( handle ); fileDeleted = GetFileAttributesA( buffer ) == INVALID_FILE_ATTRIBUTES && GetLastError() == ERROR_FILE_NOT_FOUND; - todo_wine ok( !fileDeleted, "Directory shouldn't have been deleted\n" ); fileDeleted = RemoveDirectoryA( buffer ); -todo_wine ok( fileDeleted, "Directory should have been deleted\n" ); }
diff --git a/server/fd.c b/server/fd.c index 06d1d81bdb..e75d489748 100644 --- a/server/fd.c +++ b/server/fd.c @@ -93,6 +93,9 @@ #ifdef HAVE_SYS_SYSCALL_H #include <sys/syscall.h> #endif +#ifdef HAVE_DIRENT_H +#include <dirent.h> +#endif
#include "ntstatus.h" #define WIN32_NO_STATUS @@ -2364,6 +2367,36 @@ static struct fd *get_handle_fd_obj( struct process *process, obj_handle_t handl return fd; }
+static int is_dir_empty( int fd ) +{ +#ifdef HAVE_DIRENT_H + DIR *dir; + int empty; + struct dirent *de; + + if ((fd = dup( fd )) == -1) + return -1; + + if (!(dir = fdopendir( fd ))) + { + close( fd ); + return -1; + } + + empty = 1; + while (empty && (de = readdir( dir ))) + { + if (!strcmp( de->d_name, "." ) || !strcmp( de->d_name, ".." )) continue; + empty = 0; + } + closedir( dir ); + close( fd ); + return empty; +#else + return 1; +#endif +} + /* set disposition for the fd */ static void set_fd_disposition( struct fd *fd, int unlink ) { @@ -2401,6 +2434,24 @@ static void set_fd_disposition( struct fd *fd, int unlink ) return; }
+ /* can't remove non-empty directories */ + if (unlink && S_ISDIR(st.st_mode)) + { + int empty; + + if ((empty = is_dir_empty( fd->unix_fd )) == -1) + { + file_set_error(); + return; + } + + if (!empty) + { + set_error( STATUS_DIRECTORY_NOT_EMPTY ); + return; + } + } + fd->closed->unlink = unlink ? 1 : 0; if (fd->options & FILE_DELETE_ON_CLOSE) fd->closed->unlink = -1;
May 28, 2020 5:40 AM, "Daniel Lehman" dlehman25@gmail.com wrote:
diff --git a/server/fd.c b/server/fd.c index 06d1d81bdb..e75d489748 100644 --- a/server/fd.c +++ b/server/fd.c @@ -2364,6 +2367,36 @@ static struct fd *get_handle_fd_obj( struct process *process, obj_handle_t handl return fd; }
+static int is_dir_empty( int fd ) +{ +#ifdef HAVE_DIRENT_H
- DIR *dir;
- int empty;
- struct dirent *de;
- if ((fd = dup( fd )) == -1)
return -1;- if (!(dir = fdopendir( fd )))
- {
close( fd );return -1;- }
- empty = 1;
- while (empty && (de = readdir( dir )))
- {
if (!strcmp( de->d_name, "." ) || !strcmp( de->d_name, ".." )) continue;empty = 0;
You could probably break from the loop after this point, since you found what you were looking for.
- }
- closedir( dir );
- close( fd );
This close(2) call shouldn't be necessary, since closedir(3) does that for you. It's the reason I suggested dup(2)'ing the fd in the first place. Leaving this in will set errno to EBADF, which may not be desirable.
- return empty;
+#else
- return 1;
+#endif +}
/* set disposition for the fd */ static void set_fd_disposition( struct fd *fd, int unlink ) {
Chip
- empty = 1;
- while (empty && (de = readdir( dir )))
- {
if (!strcmp( de->d_name, "." ) || !strcmp( de->d_name, ".." )) continue;empty = 0;You could probably break from the loop after this point, since you found what you were looking for.
it does break the loop. i originally had an explicit break, but changed it to break when empty is zeroed. i can switch it back if preferred
- }
- closedir( dir );
- close( fd );
This close(2) call shouldn't be necessary, since closedir(3) does that for you. It's the reason I suggested dup(2)'ing the fd in the first place. Leaving this in will set errno to EBADF, which may not be desirable.
i had considered that, but the documentation suggests that using a file descriptor is implementation-dependent. i know that's the case for linux, but not sure about the others. from docs, it looks like freebsd, macosx, and android are the same. if that covers it, i'll remove the close
thanks daniel
May 28, 2020 7:44 PM, "Daniel Lehman" dlehman25@gmail.com wrote:
- empty = 1;
- while (empty && (de = readdir( dir )))
- {
- if (!strcmp( de->d_name, "." ) || !strcmp( de->d_name, ".." )) continue;
- empty = 0;
You could probably break from the loop after this point, since you found what you were looking for.
it does break the loop. i originally had an explicit break, but changed it to break when empty is zeroed. i can switch it back if preferred
Ah, I see now. Sorry about that. It should be fine as is.
- }
- closedir( dir );
- close( fd );
This close(2) call shouldn't be necessary, since closedir(3) does that for you. It's the reason I suggested dup(2)'ing the fd in the first place. Leaving this in will set errno to EBADF, which may not be desirable.
i had considered that, but the documentation suggests that using a file descriptor is implementation-dependent. i know that's the case for linux, but not sure about the others. from docs, it looks like freebsd, macosx, and android are the same. if that covers it, i'll remove the close
Let's see what POSIX has to say:
Upon calling closedir() the file descriptor shall be closed.[1]
Most modern systems should conform to POSIX. I think you should be OK removing the close(2) call.
[1]: https://pubs.opengroup.org/onlinepubs/9699919799/functions/fdopendir.html#
Chip
Let's see what POSIX has to say:
Upon calling closedir() the file descriptor shall be closed.[1]
Most modern systems should conform to POSIX. I think you should be OK removing the close(2) call.
ah... i was looking at an older version of the docs. will do
thanks daniel
-
Chip Davis -
Daniel Lehman