dll: revert the Delete redesign

inc/winfsp/winfsp.h

@@ -47,19 +47,6 @@
 extern "C" {
 #endif
 
-/*
- * The FILE_DISPOSITION_* definitions appear to be missing from the user mode headers.
- */
-#if !defined(FILE_DISPOSITION_DELETE)
-#define FILE_DISPOSITION_DO_NOT_DELETE  0x00000000
-#define FILE_DISPOSITION_DELETE         0x00000001
-#define FILE_DISPOSITION_POSIX_SEMANTICS 0x00000002
-/* remaining flags are not needed for user mode file systems but included for completeness */
-#define FILE_DISPOSITION_FORCE_IMAGE_SECTION_CHECK 0x00000004
-#define FILE_DISPOSITION_ON_CLOSE       0x00000008
-#define FILE_DISPOSITION_IGNORE_READONLY_ATTRIBUTE 0x00000010
-#endif
-
 /*
  * The REPARSE_DATA_BUFFER definitions appear to be missing from the user mode headers.
  */
@@ -361,9 +348,6 @@ typedef struct _FSP_FILE_SYSTEM_INTERFACE
     /**
      * Cleanup a file.
      *
-     * (NOTE: use of this function with the FspCleanupDelete flag is not recommended;
-     * use Delete instead.)
-     *
      * When CreateFile is used to open or create a file the kernel creates a kernel mode file
      * object (type FILE_OBJECT) and a handle for it, which it returns to user-mode. The handle may
      * be duplicated (using DuplicateHandle), but all duplicate handles always refer to the same
@@ -418,7 +402,6 @@ typedef struct _FSP_FILE_SYSTEM_INTERFACE
      *     Close
      *     CanDelete
      *     SetDelete
-     *     Delete
      */
     VOID (*Cleanup)(FSP_FILE_SYSTEM *FileSystem,
         PVOID FileContext, PWSTR FileName, ULONG Flags);
@@ -592,8 +575,6 @@ typedef struct _FSP_FILE_SYSTEM_INTERFACE
     /**
      * Determine whether a file or directory can be deleted.
      *
-     * (NOTE: use of this function is not recommended; use Delete instead.)
-     *
      * This function tests whether a file or directory can be safely deleted. This function does
      * not need to perform access checks, but may performs tasks such as check for empty
      * directories, etc.
@@ -618,7 +599,6 @@ typedef struct _FSP_FILE_SYSTEM_INTERFACE
      * @see
      *     Cleanup
      *     SetDelete
-     *     Delete
      */
     NTSTATUS (*CanDelete)(FSP_FILE_SYSTEM *FileSystem,
         PVOID FileContext, PWSTR FileName);
@@ -900,8 +880,6 @@ typedef struct _FSP_FILE_SYSTEM_INTERFACE
     /**
      * Set the file delete flag.
      *
-     * (NOTE: use of this function is not recommended; use Delete instead.)
-     *
      * This function sets a flag to indicates whether the FSD file should delete a file
      * when it is closed. This function does not need to perform access checks, but may
      * performs tasks such as check for empty directories, etc.
@@ -930,7 +908,6 @@ typedef struct _FSP_FILE_SYSTEM_INTERFACE
      * @see
      *     Cleanup
      *     CanDelete
-     *     Delete
      */
     NTSTATUS (*SetDelete)(FSP_FILE_SYSTEM *FileSystem,
         PVOID FileContext, PWSTR FileName, BOOLEAN DeleteFile);
@@ -1063,59 +1040,8 @@ typedef struct _FSP_FILE_SYSTEM_INTERFACE
         PVOID FileContext,
         PFILE_FULL_EA_INFORMATION Ea, ULONG EaLength,
         FSP_FSCTL_FILE_INFO *FileInfo);
-    /**
+
-     * Set the file delete flag or delete a file.
+    NTSTATUS (*Obsolete0)(VOID);
-     *
-     * This function replaces CanDelete, SetDelete and uses of Cleanup with the FspCleanupDelete flag
-     * and is recommended for use in all new code.
-     *
-     * Due to the complexity of file deletion in the Windows file system this function is used
-     * in many scenarios. Its usage is controlled by the Flags parameter:
-     * <ul>
-     * <li>FILE_DISPOSITION_DO_NOT_DELETE: Unmark the file for deletion.
-     * Do <b>NOT</b> delete the file either now or at Cleanup time.</li>
-     * <li>FILE_DISPOSITION_DELETE: Mark the file for deletion,
-     * but do <b>NOT</b> delete the file. The file will be deleted at Cleanup time
-     * (via a call to Delete with Flags = -1).
-     * This function does not need to perform access checks, but may
-     * performs tasks such as check for empty directories, etc.</li>
-     * <li>FILE_DISPOSITION_DELETE | FILE_DISPOSITION_POSIX_SEMANTICS: Delete the file
-     * <b>NOW</b> using POSIX semantics. Open user mode handles to the file remain valid.
-     * This case will be received only when FSP_FSCTL_VOLUME_PARAMS :: SupportsPosixUnlinkRename is set.</li>
-     * <li>-1: Delete the file <b>NOW</b> using regular Windows semantics.
-     * Called during Cleanup with no open user mode handles remaining.
-     * If a file system implements Delete, Cleanup should <b>NOT</b> be used for deletion anymore.</li>
-     * </ul>
-     *
-     * This function gets called in all file deletion scenarios:
-     * <ul>
-     * <li>When the DeleteFile or RemoveDirectory API's are used.</li>
-     * <li>When the SetInformationByHandle API with FileDispositionInfo or FileDispositionInfoEx is used.</li>
-     * <li>When a file is opened using FILE_DELETE_ON_CLOSE.</li>
-     * <li>Etc.</li>
-     * </ul>
-     *
-     * NOTE: Delete takes precedence over CanDelete, SetDelete and Cleanup with the FspCleanupDelete flag.
-     * This means that if Delete is defined, CanDelete and SetDelete will never be called and
-     * Cleanup will never be called with the FspCleanupDelete flag.
-     *
-     * @param FileSystem
-     *     The file system on which this request is posted.
-     * @param FileContext
-     *     The file context of the file or directory to set the delete flag for.
-     * @param FileName
-     *     The name of the file or directory to set the delete flag for.
-     * @param Flags
-     *     File disposition flags
-     * @return
-     *     STATUS_SUCCESS or error code.
-     * @see
-     *     Cleanup
-     *     CanDelete
-     *     SetDelete
-     */
-    NTSTATUS (*Delete)(FSP_FILE_SYSTEM *FileSystem,
-        PVOID FileContext, PWSTR FileName, ULONG Flags);
 
     /*
      * This ensures that this interface will always contain 64 function pointers.

src/dll/fsop.c

@@ -57,9 +57,7 @@ FSP_API NTSTATUS FspFileSystemOpEnter(FSP_FILE_SYSTEM *FileSystem,
                 Request->Req.Cleanup.Delete) ||
             (FspFsctlTransactSetInformationKind == Request->Kind &&
                 (10/*FileRenameInformation*/ == Request->Req.SetInformation.FileInformationClass ||
-                65/*FileRenameInformationEx*/ == Request->Req.SetInformation.FileInformationClass ||
+                65/*FileRenameInformationEx*/ == Request->Req.SetInformation.FileInformationClass)) ||
-                (64/*FileDispositionInformationEx*/ == Request->Req.SetInformation.FileInformationClass &&
-                    3/*DELETE|POSIX_SEMANTICS*/ == (3 & Request->Req.SetInformation.Info.DispositionEx.Flags)))) ||
             FspFsctlTransactSetVolumeInformationKind == Request->Kind ||
             (FspFsctlTransactFlushBuffersKind == Request->Kind &&
                 0 == Request->Req.FlushBuffers.UserContext &&
@@ -71,8 +69,7 @@ FSP_API NTSTATUS FspFileSystemOpEnter(FSP_FILE_SYSTEM *FileSystem,
         if (FspFsctlTransactCreateKind == Request->Kind ||
             (FspFsctlTransactSetInformationKind == Request->Kind &&
                 (13/*FileDispositionInformation*/ == Request->Req.SetInformation.FileInformationClass ||
-                (64/*FileDispositionInformationEx*/ == Request->Req.SetInformation.FileInformationClass &&
+                64/*FileDispositionInformationEx*/ == Request->Req.SetInformation.FileInformationClass)) ||
-                    3/*DELETE|POSIX_SEMANTICS*/ != (3 & Request->Req.SetInformation.Info.DispositionEx.Flags)))) ||
             FspFsctlTransactQueryDirectoryKind == Request->Kind ||
             FspFsctlTransactQueryVolumeInformationKind == Request->Kind)
         {
@@ -101,9 +98,7 @@ FSP_API NTSTATUS FspFileSystemOpLeave(FSP_FILE_SYSTEM *FileSystem,
                 Request->Req.Cleanup.Delete) ||
             (FspFsctlTransactSetInformationKind == Request->Kind &&
                 (10/*FileRenameInformation*/ == Request->Req.SetInformation.FileInformationClass ||
-                65/*FileRenameInformationEx*/ == Request->Req.SetInformation.FileInformationClass ||
+                65/*FileRenameInformationEx*/ == Request->Req.SetInformation.FileInformationClass)) ||
-                (64/*FileDispositionInformationEx*/ == Request->Req.SetInformation.FileInformationClass &&
-                    3/*DELETE|POSIX_SEMANTICS*/ == (3 & Request->Req.SetInformation.Info.DispositionEx.Flags)))) ||
             FspFsctlTransactSetVolumeInformationKind == Request->Kind ||
             (FspFsctlTransactFlushBuffersKind == Request->Kind &&
                 0 == Request->Req.FlushBuffers.UserContext &&
@@ -115,8 +110,7 @@ FSP_API NTSTATUS FspFileSystemOpLeave(FSP_FILE_SYSTEM *FileSystem,
         if (FspFsctlTransactCreateKind == Request->Kind ||
             (FspFsctlTransactSetInformationKind == Request->Kind &&
                 (13/*FileDispositionInformation*/ == Request->Req.SetInformation.FileInformationClass ||
-                (64/*FileDispositionInformationEx*/ == Request->Req.SetInformation.FileInformationClass &&
+                64/*FileDispositionInformationEx*/ == Request->Req.SetInformation.FileInformationClass)) ||
-                    3/*DELETE|POSIX_SEMANTICS*/ != (3 & Request->Req.SetInformation.Info.DispositionEx.Flags)))) ||
             FspFsctlTransactQueryDirectoryKind == Request->Kind ||
             FspFsctlTransactQueryVolumeInformationKind == Request->Kind)
         {
@@ -991,28 +985,16 @@ FSP_API NTSTATUS FspFileSystemOpOverwrite(FSP_FILE_SYSTEM *FileSystem,
 FSP_API NTSTATUS FspFileSystemOpCleanup(FSP_FILE_SYSTEM *FileSystem,
     FSP_FSCTL_TRANSACT_REQ *Request, FSP_FSCTL_TRANSACT_RSP *Response)
 {
-    ULONG CleanupFlags =
-        (0 != Request->Req.Cleanup.Delete ? FspCleanupDelete : 0) |
-        (0 != Request->Req.Cleanup.SetAllocationSize ? FspCleanupSetAllocationSize : 0) |
-        (0 != Request->Req.Cleanup.SetArchiveBit ? FspCleanupSetArchiveBit : 0) |
-        (0 != Request->Req.Cleanup.SetLastAccessTime ? FspCleanupSetLastAccessTime : 0) |
-        (0 != Request->Req.Cleanup.SetLastWriteTime ? FspCleanupSetLastWriteTime : 0) |
-        (0 != Request->Req.Cleanup.SetChangeTime ? FspCleanupSetChangeTime : 0);
-
-    if (Request->Req.Cleanup.Delete && 0 != FileSystem->Interface->Delete)
-    {
-        FileSystem->Interface->Delete(FileSystem,
-            (PVOID)ValOfFileContext(Request->Req.Cleanup),
-            0 != Request->FileName.Size ? (PWSTR)Request->Buffer : 0,
-            (ULONG)-1);
-        CleanupFlags &= ~FspCleanupDelete;
-    }
-
     if (0 != FileSystem->Interface->Cleanup)
         FileSystem->Interface->Cleanup(FileSystem,
             (PVOID)ValOfFileContext(Request->Req.Cleanup),
             0 != Request->FileName.Size ? (PWSTR)Request->Buffer : 0,
-            CleanupFlags);
+            (0 != Request->Req.Cleanup.Delete ? FspCleanupDelete : 0) |
+            (0 != Request->Req.Cleanup.SetAllocationSize ? FspCleanupSetAllocationSize : 0) |
+            (0 != Request->Req.Cleanup.SetArchiveBit ? FspCleanupSetArchiveBit : 0) |
+            (0 != Request->Req.Cleanup.SetLastAccessTime ? FspCleanupSetLastAccessTime : 0) |
+            (0 != Request->Req.Cleanup.SetLastWriteTime ? FspCleanupSetLastWriteTime : 0) |
+            (0 != Request->Req.Cleanup.SetChangeTime ? FspCleanupSetChangeTime : 0));
 
     return STATUS_SUCCESS;
 }
@@ -1171,17 +1153,7 @@ FSP_API NTSTATUS FspFileSystemOpSetInformation(FSP_FILE_SYSTEM *FileSystem,
                 break;
             }
         }
-        if (0 != FileSystem->Interface->Delete)
+        if (0 != FileSystem->Interface->SetDelete)
-        {
-            Result = FileSystem->Interface->Delete(FileSystem,
-                (PVOID)ValOfFileContext(Request->Req.SetInformation),
-                (PWSTR)Request->Buffer,
-                Request->Req.SetInformation.Info.DispositionEx.Flags & 3/*DELETE|POSIX_SEMANTICS*/);
-        }
-        else if (0 != (2/*POSIX_SEMANTICS*/ & Request->Req.SetInformation.Info.DispositionEx.Flags))
-            /* only FSP_FILE_SYSTEM_INTERFACE::Delete can do POSIX semantics; return error if unimplemented */
-            Result = STATUS_INVALID_PARAMETER;
-        else if (0 != FileSystem->Interface->SetDelete)
         {
             Result = FileSystem->Interface->SetDelete(FileSystem,
                 (PVOID)ValOfFileContext(Request->Req.SetInformation),

tst/memfs/memfs.cpp

@@ -88,13 +88,6 @@ FSP_FSCTL_STATIC_ASSERT(MEMFS_MAX_PATH > MAX_PATH,
 #define MEMFS_REJECT_EARLY_IRP
 #endif
 
-/*
- * Define the MEMFS_DELETE macro to include new Delete support
- * (instead of Cleanup/FspCleanupDelete). This is required to
- * properly support POSIX unlink/rename.
- */
-#define MEMFS_DELETE
-
 /*
  * Define the DEBUG_BUFFER_CHECK macro on Windows 8 or above. This includes
  * a check for the Write buffer to ensure that it is read-only.
@@ -972,8 +965,6 @@ void SlowioReadDirectoryThread(
  * FSP_FILE_SYSTEM_INTERFACE
  */
 
-static NTSTATUS Delete(FSP_FILE_SYSTEM *FileSystem,
-    PVOID FileNode0, PWSTR FileName, ULONG Flags);
 #if defined(MEMFS_REPARSE_POINTS)
 static NTSTATUS GetReparsePointByName(
     FSP_FILE_SYSTEM *FileSystem, PVOID Context,
@@ -1356,9 +1347,7 @@ static VOID Cleanup(FSP_FILE_SYSTEM *FileSystem,
     MEMFS_FILE_NODE *MainFileNode = FileNode;
 #endif
 
-#if !defined(MEMFS_DELETE)
     assert(0 != Flags); /* FSP_FSCTL_VOLUME_PARAMS::PostCleanupWhenModifiedOnly ensures this */
-#endif
 
     if (Flags & FspCleanupSetArchiveBit)
     {
@@ -1387,10 +1376,21 @@ static VOID Cleanup(FSP_FILE_SYSTEM *FileSystem,
         SetFileSizeInternal(FileSystem, FileNode, AllocationSize, TRUE);
     }
 
-#if !defined(MEMFS_DELETE)
+    if ((Flags & FspCleanupDelete) && !MemfsFileNodeMapHasChild(Memfs->FileNodeMap, FileNode))
-    if (Flags & FspCleanupDelete)
+    {
-        Delete(FileSystem, FileNode0, FileName, -1);
+#if defined(MEMFS_NAMED_STREAMS)
+        MEMFS_FILE_NODE_MAP_ENUM_CONTEXT Context = { FALSE };
+        ULONG Index;
+
+        MemfsFileNodeMapEnumerateNamedStreams(Memfs->FileNodeMap, FileNode,
+            MemfsFileNodeMapEnumerateFn, &Context);
+        for (Index = 0; Context.Count > Index; Index++)
+            MemfsFileNodeMapRemove(Memfs->FileNodeMap, Context.FileNodes[Index]);
+        MemfsFileNodeMapEnumerateFree(&Context);
 #endif
+
+        MemfsFileNodeMapRemove(Memfs->FileNodeMap, FileNode);
+    }
 }
 
 static VOID Close(FSP_FILE_SYSTEM *FileSystem,
@@ -1651,13 +1651,17 @@ static NTSTATUS SetFileSize(FSP_FILE_SYSTEM *FileSystem,
     return STATUS_SUCCESS;
 }
 
-#if !defined(MEMFS_DELETE)
 static NTSTATUS CanDelete(FSP_FILE_SYSTEM *FileSystem,
     PVOID FileNode0, PWSTR FileName)
 {
-    return Delete(FileSystem, FileNode0, FileName, FILE_DISPOSITION_DELETE);
+    MEMFS *Memfs = (MEMFS *)FileSystem->UserContext;
+    MEMFS_FILE_NODE *FileNode = (MEMFS_FILE_NODE *)FileNode0;
+
+    if (MemfsFileNodeMapHasChild(Memfs->FileNodeMap, FileNode))
+        return STATUS_DIRECTORY_NOT_EMPTY;
+
+    return STATUS_SUCCESS;
 }
-#endif
 
 static NTSTATUS Rename(FSP_FILE_SYSTEM *FileSystem,
     PVOID FileNode0,
@@ -2227,54 +2231,6 @@ static NTSTATUS SetEa(FSP_FILE_SYSTEM *FileSystem,
 }
 #endif
 
-static NTSTATUS Delete(FSP_FILE_SYSTEM *FileSystem,
-    PVOID FileNode0, PWSTR FileName, ULONG Flags)
-{
-    MEMFS *Memfs = (MEMFS *)FileSystem->UserContext;
-    MEMFS_FILE_NODE *FileNode = (MEMFS_FILE_NODE *)FileNode0;
-
-    switch (Flags)
-    {
-    case FILE_DISPOSITION_DO_NOT_DELETE:
-        // set file disposition flag: do not delete file at Cleanup
-        return STATUS_SUCCESS;
-
-    case FILE_DISPOSITION_DELETE:
-        // set file disposition flag: delete file at Cleanup
-        if (MemfsFileNodeMapHasChild(Memfs->FileNodeMap, FileNode))
-            return STATUS_DIRECTORY_NOT_EMPTY;
-        return STATUS_SUCCESS;
-
-    case FILE_DISPOSITION_DELETE | FILE_DISPOSITION_POSIX_SEMANTICS:
-        // delete file now; open handles to file remain valid
-        /* fallthrough */
-
-    case -1:
-        // delete file now; called during Cleanup
-        if (MemfsFileNodeMapHasChild(Memfs->FileNodeMap, FileNode))
-            return STATUS_DIRECTORY_NOT_EMPTY;
-        else
-        {
-#if defined(MEMFS_NAMED_STREAMS)
-            MEMFS_FILE_NODE_MAP_ENUM_CONTEXT Context = { FALSE };
-            ULONG Index;
-
-            MemfsFileNodeMapEnumerateNamedStreams(Memfs->FileNodeMap, FileNode,
-                MemfsFileNodeMapEnumerateFn, &Context);
-            for (Index = 0; Context.Count > Index; Index++)
-                MemfsFileNodeMapRemove(Memfs->FileNodeMap, Context.FileNodes[Index]);
-            MemfsFileNodeMapEnumerateFree(&Context);
-#endif
-
-            MemfsFileNodeMapRemove(Memfs->FileNodeMap, FileNode);
-            return STATUS_SUCCESS;
-        }
-
-    default:
-        return STATUS_INVALID_PARAMETER;
-    }
-}
-
 static FSP_FILE_SYSTEM_INTERFACE MemfsInterface =
 {
     GetVolumeInfo,
@@ -2299,11 +2255,7 @@ static FSP_FILE_SYSTEM_INTERFACE MemfsInterface =
     GetFileInfo,
     SetBasicInfo,
     SetFileSize,
-#if !defined(MEMFS_DELETE)
     CanDelete,
-#else
-    0,
-#endif
     Rename,
     GetSecurity,
     SetSecurity,
@@ -2347,11 +2299,6 @@ static FSP_FILE_SYSTEM_INTERFACE MemfsInterface =
     0,
     0,
 #endif
-#if defined(MEMFS_DELETE)
-    Delete,
-#else
-    0,
-#endif
 };
 
 /*

tst/winfsp-tests/exec-test.c

@@ -209,7 +209,7 @@ static void exec_delete_dotest(ULONG Flags, PWSTR Prefix, ULONG FileInfoTimeout)
             OPEN_EXISTING, 0, 0);
         if (INVALID_HANDLE_VALUE != Handle)
         {
-            DispositionInfo.Flags = FILE_DISPOSITION_DELETE | FILE_DISPOSITION_POSIX_SEMANTICS;
+            DispositionInfo.Flags = 3/*FILE_DISPOSITION_DELETE | FILE_DISPOSITION_POSIX_SEMANTICS*/;
             Success = SetFileInformationByHandle(Handle,
                 21/*FileDispositionInfoEx*/, &DispositionInfo, sizeof DispositionInfo);
             ASSERT(!Success);