https://git.reactos.org/?p=reactos.git;a=commitdiff;h=829ad06179a8710392997e...
commit 829ad06179a8710392997efb0e9464224e999d8e Author: Hermès Bélusca-Maïto hermes.belusca-maito@reactos.org AuthorDate: Thu Sep 26 20:31:19 2024 +0200 Commit: Hermès Bélusca-Maïto hermes.belusca-maito@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); }