[ros-diffs] [reactos] 02/04: [FREELDR] fs.c: Simplify FileId checks; add missing DeviceId invalidation (#7385)

[Hermès Bélusca-Maïto]

https://git.reactos.org/?p=reactos.git;a=commitdiff;h=829ad06179a8710392997…

commit 829ad06179a8710392997efb0e9464224e999d8e
Author:     Hermès Bélusca-Maïto <hermes.belusca-maito(a)reactos.org>
AuthorDate: Thu Sep 26 20:31:19 2024 +0200
Commit:     Hermès Bélusca-Maïto <hermes.belusca-maito(a)reactos.org>
CommitDate: Tue Oct 1 22:14:01 2024 +0200

    [FREELDR] fs.c: Simplify FileId checks; add missing DeviceId invalidation (#7385)
    
    - Replace a lot of `MAX_FDS` by `_countof(FileData)`;
    
    - The duplicated FileId validation logic is wrapped with
      the `IS_VALID_FILEID()` macro.
    
    - When returning an invalid FileId value on purpose, use
      `INVALID_FILE_ID` instead of `MAX_FDS` (that could vary
      if the file handle table gets extended).
      And replace the `(ULONG)-1` also used for that purpose
      by `INVALID_FILE_ID`.
    
    - Add missing DeviceId invalidation:
      * when failing to open a file handle in `ArcOpen()`;
      * when registering a new device in `FsRegisterDevice()`.
        There, having DeviceId always set to zero would tell
        the code that the corresponding device's file ID is
        the first one in the table, which is a BUG. (Many devices
        would have the same file ID...)
    
    - In addition: massage a bit some indicial for-loops.
---
 boot/freeldr/freeldr/include/fs.h |  1 +
 boot/freeldr/freeldr/lib/fs/fs.c  | 53 +++++++++++++++++++++------------------
 2 files changed, 30 insertions(+), 24 deletions(-)

diff --git a/boot/freeldr/freeldr/include/fs.h b/boot/freeldr/freeldr/include/fs.h
index 68489c9d087..b3f90bc2a44 100644
--- a/boot/freeldr/freeldr/include/fs.h
+++ b/boot/freeldr/freeldr/include/fs.h
@@ -32,6 +32,7 @@ typedef struct tagDEVVTBL
 } DEVVTBL;
 
 #define MAX_FDS 60
+#define INVALID_FILE_ID ((ULONG)-1)
 
 ARC_STATUS ArcOpen(CHAR* Path, OPENMODE OpenMode, ULONG* FileId);
 ARC_STATUS ArcClose(ULONG FileId);
diff --git a/boot/freeldr/freeldr/lib/fs/fs.c b/boot/freeldr/freeldr/lib/fs/fs.c
index 198982b0515..33e21a609f8 100644
--- a/boot/freeldr/freeldr/lib/fs/fs.c
+++ b/boot/freeldr/freeldr/lib/fs/fs.c
@@ -51,6 +51,9 @@ typedef struct tagDEVICE
 static FILEDATA FileData[MAX_FDS];
 static LIST_ENTRY DeviceListHead;
 
+#define IS_VALID_FILEID(FileId) \
+    ((ULONG)(FileId) < _countof(FileData) &&
FileData[(ULONG)(FileId)].FuncTable)
+
 typedef const DEVVTBL* (*PFS_MOUNT)(ULONG DeviceId);
 
 PFS_MOUNT FileSystems[] =
@@ -91,7 +94,7 @@ ARC_STATUS ArcOpen(CHAR* Path, OPENMODE OpenMode, ULONG* FileId)
     /* Print status message */
     TRACE("Opening file '%s'...\n", Path);
 
-    *FileId = MAX_FDS;
+    *FileId = INVALID_FILE_ID;
 
     /* Search last ')', which delimits device and path */
     FileName = strrchr(Path, ')');
@@ -138,17 +141,17 @@ ARC_STATUS ArcOpen(CHAR* Path, OPENMODE OpenMode, ULONG* FileId)
         pDevice = CONTAINING_RECORD(pEntry, DEVICE, ListEntry);
         if (strncmp(pDevice->Prefix, DeviceName, Length) == 0)
         {
-            /* OK, device found. It is already opened? */
+            /* OK, device found. Is it already opened? */
             if (pDevice->ReferenceCount == 0)
             {
-                /* Search some room for the device */
-                for (DeviceId = 0; DeviceId < MAX_FDS; DeviceId++)
+                /* Find some room for the device */
+                for (DeviceId = 0; ; ++DeviceId)
                 {
+                    if (DeviceId >= _countof(FileData))
+                        return EMFILE;
                     if (!FileData[DeviceId].FuncTable)
                         break;
                 }
-                if (DeviceId == MAX_FDS)
-                    return EMFILE;
 
                 /* Try to open the device */
                 FileData[DeviceId].FuncTable = pDevice->FuncTable;
@@ -199,14 +202,14 @@ ARC_STATUS ArcOpen(CHAR* Path, OPENMODE OpenMode, ULONG* FileId)
      * in DeviceId, and FileData[DeviceId].FileFuncTable contains what
      * needs to be called to open the file */
 
-    /* Search some room for the device */
-    for (i = 0; i < MAX_FDS; i++)
+    /* Find some room for the file */
+    for (i = 0; ; ++i)
     {
+        if (i >= _countof(FileData))
+            return EMFILE;
         if (!FileData[i].FuncTable)
             break;
     }
-    if (i == MAX_FDS)
-        return EMFILE;
 
     /* Skip leading path separator, if any */
     if (*FileName == '\\' || *FileName == '/')
@@ -219,8 +222,10 @@ ARC_STATUS ArcOpen(CHAR* Path, OPENMODE OpenMode, ULONG* FileId)
     Status = FileData[i].FuncTable->Open(FileName, OpenMode, FileId);
     if (Status != ESUCCESS)
     {
+        FileData[i].DeviceId = INVALID_FILE_ID;
         FileData[i].FuncTable = NULL;
-        *FileId = MAX_FDS;
+        FileData[i].Specific = NULL;
+        *FileId = INVALID_FILE_ID;
     }
     return Status;
 }
@@ -229,37 +234,36 @@ ARC_STATUS ArcClose(ULONG FileId)
 {
     ARC_STATUS Status;
 
-    if (FileId >= MAX_FDS || !FileData[FileId].FuncTable)
+    if (!IS_VALID_FILEID(FileId))
         return EBADF;
 
     Status = FileData[FileId].FuncTable->Close(FileId);
-
     if (Status == ESUCCESS)
     {
+        FileData[FileId].DeviceId = INVALID_FILE_ID;
         FileData[FileId].FuncTable = NULL;
         FileData[FileId].Specific = NULL;
-        FileData[FileId].DeviceId = -1;
     }
     return Status;
 }
 
 ARC_STATUS ArcRead(ULONG FileId, VOID* Buffer, ULONG N, ULONG* Count)
 {
-    if (FileId >= MAX_FDS || !FileData[FileId].FuncTable)
+    if (!IS_VALID_FILEID(FileId))
         return EBADF;
     return FileData[FileId].FuncTable->Read(FileId, Buffer, N, Count);
 }
 
 ARC_STATUS ArcSeek(ULONG FileId, LARGE_INTEGER* Position, SEEKMODE SeekMode)
 {
-    if (FileId >= MAX_FDS || !FileData[FileId].FuncTable)
+    if (!IS_VALID_FILEID(FileId))
         return EBADF;
     return FileData[FileId].FuncTable->Seek(FileId, Position, SeekMode);
 }
 
 ARC_STATUS ArcGetFileInformation(ULONG FileId, FILEINFORMATION* Information)
 {
-    if (FileId >= MAX_FDS || !FileData[FileId].FuncTable)
+    if (!IS_VALID_FILEID(FileId))
         return EBADF;
     return FileData[FileId].FuncTable->GetFileInformation(FileId, Information);
 }
@@ -402,6 +406,7 @@ VOID FsRegisterDevice(CHAR* Prefix, const DEVVTBL* FuncTable)
     if (!pNewEntry)
         return;
     pNewEntry->FuncTable = FuncTable;
+    pNewEntry->DeviceId = INVALID_FILE_ID;
     pNewEntry->ReferenceCount = 0;
     pNewEntry->Prefix = (CHAR*)(pNewEntry + 1);
     RtlCopyMemory(pNewEntry->Prefix, Prefix, Length);
@@ -411,29 +416,29 @@ VOID FsRegisterDevice(CHAR* Prefix, const DEVVTBL* FuncTable)
 
 PCWSTR FsGetServiceName(ULONG FileId)
 {
-    if (FileId >= MAX_FDS || !FileData[FileId].FuncTable)
+    if (!IS_VALID_FILEID(FileId))
         return NULL;
     return FileData[FileId].FuncTable->ServiceName;
 }
 
 VOID FsSetDeviceSpecific(ULONG FileId, VOID* Specific)
 {
-    if (FileId >= MAX_FDS || !FileData[FileId].FuncTable)
+    if (!IS_VALID_FILEID(FileId))
         return;
     FileData[FileId].Specific = Specific;
 }
 
 VOID* FsGetDeviceSpecific(ULONG FileId)
 {
-    if (FileId >= MAX_FDS || !FileData[FileId].FuncTable)
+    if (!IS_VALID_FILEID(FileId))
         return NULL;
     return FileData[FileId].Specific;
 }
 
 ULONG FsGetDeviceId(ULONG FileId)
 {
-    if (FileId >= MAX_FDS)
-        return (ULONG)-1;
+    if (FileId >= _countof(FileData)) // !IS_VALID_FILEID(FileId)
+        return INVALID_FILE_ID;
     return FileData[FileId].DeviceId;
 }
 
@@ -442,8 +447,8 @@ VOID FsInit(VOID)
     ULONG i;
 
     RtlZeroMemory(FileData, sizeof(FileData));
-    for (i = 0; i < MAX_FDS; i++)
-        FileData[i].DeviceId = (ULONG)-1;
+    for (i = 0; i < _countof(FileData); ++i)
+        FileData[i].DeviceId = INVALID_FILE_ID;
 
     InitializeListHead(&DeviceListHead);
 }