From aa9354773b28621aa02816cd868fd200de83e6d2 Mon Sep 17 00:00:00 2001 From: John Oberschelp Date: Tue, 10 Sep 2019 15:58:35 -0700 Subject: [PATCH 1/3] Fixed issues noted at the PR review --- tst/airfs/airfs.cpp | 35 ++++++++++++++++------------------- 1 file changed, 16 insertions(+), 19 deletions(-) diff --git a/tst/airfs/airfs.cpp b/tst/airfs/airfs.cpp index 1b1ea50d..466aea8a 100644 --- a/tst/airfs/airfs.cpp +++ b/tst/airfs/airfs.cpp @@ -1139,7 +1139,7 @@ void AirfsDelete(AIRFS_ Airfs) ////////////////////////////////////////////////////////////////////// NTSTATUS AirfsCreate( - PWSTR VolumeName, + PWSTR StorageFileName, PWSTR MapName, ULONG Flags, ULONG FileInfoTimeout, @@ -1160,12 +1160,12 @@ NTSTATUS AirfsCreate( *PAirfs = 0; - boolean VolumeExists = *VolumeName && (_waccess(VolumeName, 0) != -1); + boolean StorageFileExists = *StorageFileName && (_waccess(StorageFileName, 0) != -1); - Result = StorageStartup(Airfs, MapName, VolumeName, VolumeSize); + Result = StorageStartup(Airfs, MapName, StorageFileName, VolumeSize); if (Result) return Result; - boolean ShouldFormat = !VolumeExists || memcmp(Airfs->VolumeLabel, L"AIRFS", 10); + boolean ShouldFormat = !StorageFileExists || memcmp(Airfs->Signature, "Airfs\0\0\0", 8); if (ShouldFormat) { @@ -1177,10 +1177,10 @@ NTSTATUS AirfsCreate( &RootSecurity, &RootSecuritySize)) return GetLastErrorAsStatus(); - Airfs->VolumeName[0] = 0; Airfs->VolumeSize = ROUND_DOWN(VolumeSize, ALLOCATION_UNIT); - Airfs->CaseInsensitive = CaseInsensitive; + Airfs->VolumeLabelLength = sizeof L"AIRFS" - sizeof WCHAR; + memcpy(Airfs->VolumeLabel, L"AIRFS", Airfs->VolumeLabelLength); FSP_FSCTL_VOLUME_PARAMS V; memset(&V, 0, sizeof V); @@ -1206,9 +1206,6 @@ NTSTATUS AirfsCreate( FileSystemName ? FileSystemName : L"-AIRFS"); Airfs->VolumeParams = V; - Airfs->VolumeLabelLength = sizeof L"AIRFS" - sizeof WCHAR; - memcpy(Airfs->VolumeLabel, L"AIRFS", Airfs->VolumeLabelLength); - // Set up the available storage in chunks. Airfs->FreeSize = 0; Airfs->Available = 0; @@ -1278,7 +1275,7 @@ NTSTATUS SvcStart(FSP_SERVICE *Service, ULONG argc, PWSTR *argv) ULONG Flags = AirfsDisk; ULONG OtherFlags = 0; ULONG FileInfoTimeout = INFINITE; - PWSTR VolumeName = L""; + PWSTR StorageFileName = L""; PWSTR MapName = L""; UINT64 VolumeSize = 16LL * 1024 * 1024; PWSTR FileSystemName = 0; @@ -1302,7 +1299,7 @@ NTSTATUS SvcStart(FSP_SERVICE *Service, ULONG argc, PWSTR *argv) case L'F': ARG_TO_S(FileSystemName); break; case L'i': OtherFlags = AirfsCaseInsensitive; break; case L'm': ARG_TO_S(MountPoint); break; - case L'N': ARG_TO_S(VolumeName); break; + case L'N': ARG_TO_S(StorageFileName); break; case L'n': ARG_TO_S(MapName); break; case L'S': ARG_TO_S(RootSddl); break; case L's': ARG_TO_8(VolumeSize); break; @@ -1345,7 +1342,7 @@ NTSTATUS SvcStart(FSP_SERVICE *Service, ULONG argc, PWSTR *argv) } Result = AirfsCreate( - VolumeName, + StorageFileName, MapName, Flags | OtherFlags, FileInfoTimeout, @@ -1386,12 +1383,12 @@ NTSTATUS SvcStart(FSP_SERVICE *Service, ULONG argc, PWSTR *argv) WCHAR buffer[1024]; _snwprintf_s(buffer, 1024, L"%S%S%s%S%s -t %ld -s %lld%S%s%S%s%S%s", PROGNAME, - *VolumeName ? " -N " : "", *VolumeName ? VolumeName : L"", - *MapName ? " -n " : "", *MapName ? MapName : L"", + *StorageFileName ? " -N " : "", *StorageFileName ? StorageFileName : L"", + *MapName ? " -n " : "", *MapName ? MapName : L"", FileInfoTimeout, VolumeSize, - RootSddl ? " -S " : "", RootSddl ? RootSddl : L"", - *VolumePrefix ? " -u " : "", *VolumePrefix ? VolumePrefix : L"", - MountPoint ? " -m " : "", MountPoint ? MountPoint : L""); + RootSddl ? " -S " : "", RootSddl ? RootSddl : L"", + *VolumePrefix ? " -u " : "", *VolumePrefix ? VolumePrefix : L"", + MountPoint ? " -m " : "", MountPoint ? MountPoint : L""); INFO(buffer); Service->UserContext = Airfs; @@ -1416,11 +1413,11 @@ NTSTATUS SvcStart(FSP_SERVICE *Service, ULONG argc, PWSTR *argv) " -i [case insensitive file system]\n" " -m MountPoint [X:|* (required if no UNC prefix)]\n" " -n MapName [(ex) \"Local\\Airfs\"]\n" - " -N VolumeName [\"\": in memory only]\n" + " -N StorageFileName [\"\": in memory only]\n" " -s VolumeSize [bytes]\n" " -S RootSddl [file rights: FA, etc; NO generic rights: GA, etc.]\n" " -t FileInfoTimeout [millis]\n" - " -u \\Server\\Share [UNC prefix (single backslash)]\n"; + " -u \\Server\\Share [UNC prefix (single backslash)]\n"; FAIL(usage, L"" PROGNAME); From a6800dd73dce280eb959ae95e65f70c5c5ee40af Mon Sep 17 00:00:00 2001 From: John Oberschelp Date: Tue, 10 Sep 2019 15:59:48 -0700 Subject: [PATCH 2/3] Fixed issues noted at the PR review --- tst/airfs/common.h | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/tst/airfs/common.h b/tst/airfs/common.h index 57adff35..b9f0e94a 100644 --- a/tst/airfs/common.h +++ b/tst/airfs/common.h @@ -80,7 +80,7 @@ static int64_t wcstoll_default(wchar_t *w, int64_t deflt) // Where Class: This class manages an offset within our memory-mapped // volume to another location within our memory-mapped volume. Because it is // a self-relative offset, this delta is constant regardless of where in -// memory the file system is mapped, so we can always reoptain its address. +// memory the file system is mapped, so we can always reobtain its address. // A delta of 0 is the special case for "null". // @@ -130,8 +130,6 @@ typedef struct UINT16 filler1,filler2,filler3; UINT32 CaseInsensitive; UINT32 filler4; - WCHAR MapName[256]; - WCHAR VolumeName[256]; // Use "" for a memory-only page file. int64_t VolumeLength; FSP_FSCTL_VOLUME_PARAMS VolumeParams; FSP_FILE_SYSTEM *FileSystem; @@ -197,11 +195,11 @@ NODE_ Last (NODE_ start); NODE_ Next (NODE_); NODE_ Prev (NODE_); -NTSTATUS StorageStartup (AIRFS_ &, WCHAR* MapName, WCHAR* VolumeName, int64_t Length); +NTSTATUS StorageStartup (AIRFS_ &, WCHAR* MapName, WCHAR* StorageFileName, int64_t Length); NTSTATUS StorageShutdown (AIRFS_); -void* StorageAllocate (AIRFS_ Airfs, int64_t RequestedSize); -void* StorageReallocate (AIRFS_ Airfs, void* Reallocate, int64_t RequestedSize); -void StorageFree (AIRFS_ Airfs, void* Release); +void* StorageAllocate (AIRFS_, int64_t RequestedSize); +void* StorageReallocate (AIRFS_, void* Reallocate, int64_t RequestedSize); +void StorageFree (AIRFS_, void* Release); NTSTATUS StorageSetFileCapacity (AIRFS_, NODE_, int64_t MinimumRequiredCapacity); void StorageAccessFile (StorageFileAccessType, NODE_, int64_t Offset, int64_t NumBytes, char* Address); From a99fa512d40e5e9531889695ae6a2feda74756c9 Mon Sep 17 00:00:00 2001 From: John Oberschelp Date: Tue, 10 Sep 2019 16:00:34 -0700 Subject: [PATCH 3/3] Fixed issues noted at the PR review --- tst/airfs/persistence.cpp | 31 ++++++++++++++----------------- 1 file changed, 14 insertions(+), 17 deletions(-) diff --git a/tst/airfs/persistence.cpp b/tst/airfs/persistence.cpp index 85cd6d77..1bb2751f 100644 --- a/tst/airfs/persistence.cpp +++ b/tst/airfs/persistence.cpp @@ -84,11 +84,12 @@ void Airprint (const char * format, ...) // Rubbertree (because it is flexible!) // Implements a sorted set of elements, using a binary tree. // Has a function, Near, that finds nodes at or adjacent to a key. -// Has an equal branch for trees that require handling equivalent -// nodes, like Airfs' memory heap manager. // Attach, Find, and Near use splay to improve random access times. // First, Last, Next, and Prev do not, to improve sequential access times. -// Replacing Where with NODE_ would make it a pointer-based tree. +// Replacing each Where with NODE_ would make this a pointer-based tree. +// In addition to Left, Right, and Parent references, each node has an Equal +// reference that may be used to keep "equivalent" nodes. This is used by +// our memory heap manager's tree of available memory blocks, sorted by size. //-------------------------------------------------------------------- @@ -531,15 +532,15 @@ NTSTATUS StorageSetFileCapacity(AIRFS_ Airfs, NODE_ Node, int64_t minimumRequire //-------------------------------------------------------------------- -NTSTATUS StorageStartup(AIRFS_ &Airfs, WCHAR* MapName, WCHAR* VolumeName, int64_t VolumeLength) +NTSTATUS StorageStartup(AIRFS_ &Airfs, WCHAR* MapName, WCHAR* StorageFileName, int64_t VolumeLength) { HANDLE MapFileHandle = INVALID_HANDLE_VALUE; Airfs = 0; // Open. - if (*VolumeName) + if (*StorageFileName) { - MapFileHandle = CreateFileW(VolumeName, GENERIC_READ|GENERIC_WRITE|GENERIC_EXECUTE, 0, NULL, OPEN_ALWAYS, NULL, NULL); + MapFileHandle = CreateFileW(StorageFileName, GENERIC_READ|GENERIC_WRITE|GENERIC_EXECUTE, 0, NULL, OPEN_ALWAYS, NULL, NULL); if (MapFileHandle == INVALID_HANDLE_VALUE) return GetLastErrorAsStatus(); } @@ -555,8 +556,6 @@ NTSTATUS StorageStartup(AIRFS_ &Airfs, WCHAR* MapName, WCHAR* VolumeName, int64_ Airfs = (AIRFS_) MappedAddress; Airfs->MapFileHandle = MapFileHandle; Airfs->MapHandle = MapHandle; - wcscpy_s(Airfs->VolumeName, 256, VolumeName); // Use "" for a memory-only page file. - wcscpy_s(Airfs->MapName, 256, MapName); Airfs->VolumeLength = VolumeLength; return 0; @@ -571,23 +570,21 @@ NTSTATUS StorageShutdown(AIRFS_ Airfs) HANDLE M = Airfs->MapHandle; HANDLE F = Airfs->MapFileHandle; - if (Airfs) + Ok = FlushViewOfFile(Airfs, 0); if (!Ok && !Result) Result = GetLastErrorAsStatus(); + if (F != INVALID_HANDLE_VALUE) { - Ok = FlushViewOfFile(Airfs, 0); if (!Ok && !Result) Result = GetLastErrorAsStatus(); - if (F != INVALID_HANDLE_VALUE) - { - Ok = FlushFileBuffers(F); if (!Ok && !Result) Result = GetLastErrorAsStatus(); - } - Ok = UnmapViewOfFile(Airfs); if (!Ok && !Result) Result = GetLastErrorAsStatus(); + Ok = FlushFileBuffers(F); if (!Ok && !Result) Result = GetLastErrorAsStatus(); + // TODO: Set and write a flag something like Airfs->UpdatesCompleted here? } + Ok = UnmapViewOfFile(Airfs); if (!Ok && !Result) Result = GetLastErrorAsStatus(); if (M) { - Ok = CloseHandle(M); if (!Ok && !Result) Result = GetLastErrorAsStatus(); + Ok = CloseHandle(M); if (!Ok && !Result) Result = GetLastErrorAsStatus(); } if (F != INVALID_HANDLE_VALUE) { - Ok = CloseHandle(F); if (!Ok && !Result) Result = GetLastErrorAsStatus(); + Ok = CloseHandle(F); if (!Ok && !Result) Result = GetLastErrorAsStatus(); } return Result;