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