[hbelusca] 70434: [VFATLIB] It seems suspicious to directly return the value of fs_close as the NTSTATUS code of the check-disk operation (for FAT32 volumes it happens to return 1 whereas for FAT16...

Author: hbelusca
Date: Sun Dec 27 02:01:52 2015
New Revision: 70434

URL: http://svn.reactos.org/svn/reactos?rev=70434&view=rev
Log:
[VFATLIB]
It seems suspicious to directly return the value of fs_close as the NTSTATUS code of the check-disk operation (for FAT32 volumes it happens to return 1 whereas for FAT16 volumes it returns 0).
The documentation of this function says that it "returns a non-zero integer if the file system has been changed since the last fs_open, zero otherwise."
Maybe somebody has a more precise idea on that subject? In the meantime I also add some DPRINTs to attempt to diagnose the conditions where this problem occurs.

Modified:
    trunk/reactos/lib/fslib/vfatlib/vfatlib.c

Modified: trunk/reactos/lib/fslib/vfatlib/vfatlib.c
URL: http://svn.reactos.org/svn/reactos/trunk/reactos/lib/fslib/vfatlib/vfatlib.c...
==============================================================================
--- trunk/reactos/lib/fslib/vfatlib/vfatlib.c	[iso-8859-1] (original)
+++ trunk/reactos/lib/fslib/vfatlib/vfatlib.c	[iso-8859-1] Sun Dec 27 02:01:52 2015
@@ -351,6 +351,7 @@
     BOOLEAN salvage_files;
     ULONG free_clusters;
     DOS_FS fs;
+    int ret;

/* Store callback pointer */
     ChkdskCallback = Callback;
@@ -372,7 +373,11 @@
     if (CheckOnlyIfDirty && !fs_isdirty())
     {
         /* No need to check FS */
-        return fs_close(FALSE);
+        // NOTE: Returning the value of fs_close looks suspicious.
+        // return fs_close(FALSE);
+        ret = fs_close(FALSE);
+        DPRINT1("No need to check FS; fs_close returning %d\n", ret);
+        return STATUS_SUCCESS;
     }

read_boot(&fs);
@@ -404,6 +409,8 @@

if (fs_changed())
     {
+        DPRINT1("fs_changed is TRUE!\n");
+
         if (FixErrors)
         {
             if (FsCheckFlags & FSCHECK_INTERACTIVE)
@@ -430,7 +437,11 @@
     }

/* Close the volume */
-    return fs_close(FixErrors) ? STATUS_SUCCESS : STATUS_UNSUCCESSFUL;
+    // NOTE: Returning the value of fs_close looks suspicious.
+    // return fs_close(FixErrors) ? STATUS_SUCCESS : STATUS_UNSUCCESSFUL;
+    res = fs_close(FixErrors);
+    DPRINT1("fs_close returning %d\n", ret);
+    return STATUS_SUCCESS;
 }

/* EOF */