filesystem.h surface error codes (#998)

- Improvement: File copy, scan, clone, and move operations now report the underlying OS error in failure messages

1 files changed, 113 insertions, 116 deletions

diff --git a/src/zencore/filesystem.cpp b/src/zencore/filesystem.cpp
index 03e04c77c..281cb8e2e 100644
--- a/src/zencore/filesystem.cpp
+++ b/src/zencore/filesystem.cpp
@@ -1106,8 +1106,6 @@ TryCloneFile(void* SourceNativeHandle, void* TargetNativeHandle)
 	FILE_DISPOSITION_INFO FileDisposition = {TRUE};
 	if (!SetFileInformationByHandle(TargetFile, FileDispositionInfo, &FileDisposition, sizeof FileDisposition))
 	{
-		const DWORD ErrorCode = ::GetLastError();
-		SetLastError(ErrorCode);
 		return false;
 	}
 
@@ -1199,7 +1197,7 @@ TryCloneFile(void* SourceNativeHandle, void* TargetNativeHandle)
 }
 #endif	// ZEN_PLATFORM_WINDOWS
 
-bool
+std::error_code
 TryCloneFile(const std::filesystem::path& FromPath, const std::filesystem::path& ToPath)
 {
 #if ZEN_PLATFORM_WINDOWS
@@ -1213,7 +1211,7 @@ TryCloneFile(const std::filesystem::path& FromPath, const std::filesystem::path&
 	if (FromFile == INVALID_HANDLE_VALUE)
 	{
 		FromFile.Detach();
-		return false;
+		return MakeErrorCodeFromLastError();
 	}
 
 	SetFileAttributesW(ToPath.c_str(), FILE_ATTRIBUTE_NORMAL);
@@ -1229,16 +1227,20 @@ TryCloneFile(const std::filesystem::path& FromPath, const std::filesystem::path&
 	if (TargetFile == INVALID_HANDLE_VALUE)
 	{
 		TargetFile.Detach();
-		return false;
+		return MakeErrorCodeFromLastError();
 	}
-	return TryCloneFile((void*)FromFile.m_Handle, (void*)TargetFile.m_Handle);
+	if (!TryCloneFile((void*)FromFile.m_Handle, (void*)TargetFile.m_Handle))
+	{
+		return MakeErrorCodeFromLastError();
+	}
+	return {};
 
 #elif ZEN_PLATFORM_LINUX
 	// The 'from' file
 	ScopedFd FromFd(open(FromPath.c_str(), O_RDONLY | O_CLOEXEC));
 	if (!FromFd)
 	{
-		return false;
+		return MakeErrorCodeFromLastError();
 	}
 
 	// Remove any existing target so we can create a fresh clone
@@ -1248,19 +1250,20 @@ TryCloneFile(const std::filesystem::path& FromPath, const std::filesystem::path&
 	ScopedFd ToFd(open(ToPath.c_str(), O_WRONLY | O_CREAT | O_EXCL | O_CLOEXEC, 0666));
 	if (!ToFd)
 	{
-		return false;
+		return MakeErrorCodeFromLastError();
 	}
 
 	if (ioctl(ToFd.Fd, FICLONE, FromFd.Fd) != 0)
 	{
 		// Clone not supported by this filesystem or files are on different volumes.
 		// Remove the empty target file we created.
-		ToFd = ScopedFd();
+		std::error_code Ec = MakeErrorCodeFromLastError();
+		ToFd			   = ScopedFd();
 		unlink(ToPath.c_str());
-		return false;
+		return Ec;
 	}
 
-	return true;
+	return {};
 #elif ZEN_PLATFORM_MAC
 	// Remove any existing target - clonefile() requires the destination not exist
 	unlink(ToPath.c_str());
@@ -1268,70 +1271,63 @@ TryCloneFile(const std::filesystem::path& FromPath, const std::filesystem::path&
 	if (clonefile(FromPath.c_str(), ToPath.c_str(), CLONE_NOFOLLOW) != 0)
 	{
 		// Clone not supported (non-APFS) or files are on different volumes
-		return false;
+		return MakeErrorCodeFromLastError();
 	}
 
-	return true;
+	return {};
 #endif	// ZEN_PLATFORM_WINDOWS
 }
 
-void
-CopyFile(const std::filesystem::path& FromPath,
-		 const std::filesystem::path& ToPath,
-		 const CopyFileOptions&		  Options,
-		 std::error_code&			  OutErrorCode)
-{
-	OutErrorCode.clear();
-
-	bool Success = CopyFile(FromPath, ToPath, Options);
-
-	if (!Success)
-	{
-		OutErrorCode = MakeErrorCodeFromLastError();
-	}
-}
-
-bool
+std::error_code
 CopyFile(const std::filesystem::path& FromPath, const std::filesystem::path& ToPath, const CopyFileOptions& Options)
 {
-	bool Success = false;
-
 	if (Options.EnableClone)
 	{
-		Success = TryCloneFile(FromPath.native(), ToPath.native());
-		if (Success)
+		std::error_code CloneEc = TryCloneFile(FromPath.native(), ToPath.native());
+		if (CloneEc)
 		{
-			return true;
+			if (Options.MustClone)
+			{
+				ZEN_ERROR("CloneFile() failed for {} -> {}: {}", FromPath, ToPath, CloneEc.message());
+				return CloneEc;
+			}
+		}
+		else
+		{
+			return {};
 		}
 	}
-
-	if (Options.MustClone)
+	else if (Options.MustClone)
 	{
-		ZEN_ERROR("CloneFile() failed for {} -> {}", FromPath, ToPath);
-		return false;
+		ZEN_ERROR("CloneFile() required but not enabled for {} -> {}", FromPath, ToPath);
+		return std::make_error_code(std::errc::invalid_argument);
 	}
 
 #if ZEN_PLATFORM_WINDOWS
 	BOOL CancelFlag = FALSE;
-	Success			= !!::CopyFileExW(FromPath.c_str(),
-							  ToPath.c_str(),
-							  /* lpProgressRoutine */ nullptr,
-							  /* lpData */ nullptr,
-							  &CancelFlag,
-							  /* dwCopyFlags */ 0);
+	BOOL Success	= ::CopyFileExW(FromPath.c_str(),
+									ToPath.c_str(),
+									/* lpProgressRoutine */ nullptr,
+									/* lpData */ nullptr,
+									&CancelFlag,
+									/* dwCopyFlags */ 0);
+	if (!Success)
+	{
+		return MakeErrorCodeFromLastError();
+	}
 #else
 	// From file
 	ScopedFd FromFd(open(FromPath.c_str(), O_RDONLY | O_CLOEXEC));
 	if (!FromFd)
 	{
-		ThrowLastError(fmt::format("failed to open file {}", FromPath));
+		return MakeErrorCodeFromLastError();
 	}
 
 	// To file
 	ScopedFd ToFd(open(ToPath.c_str(), O_WRONLY | O_CREAT | O_CLOEXEC, 0666));
 	if (!ToFd)
 	{
-		ThrowLastError(fmt::format("failed to create file {}", ToPath));
+		return MakeErrorCodeFromLastError();
 	}
 	fchmod(ToFd.Fd, 0666);
 
@@ -1344,32 +1340,36 @@ CopyFile(const std::filesystem::path& FromPath, const std::filesystem::path& ToP
 	ZEN_UNUSED($Ignore);  // What's the appropriate error handling here?
 
 	// Copy impl
-	const size_t BufferSize = Min(FileSizeBytes, 64u << 10);
-	void*		 Buffer		= malloc(BufferSize);
+	const size_t	BufferSize = Min(FileSizeBytes, 64u << 10);
+	void*			Buffer	   = malloc(BufferSize);
+	std::error_code Result;
 	while (true)
 	{
 		int BytesRead = read(FromFd.Fd, Buffer, BufferSize);
-		if (BytesRead <= 0)
+		if (BytesRead < 0)
+		{
+			Result = MakeErrorCodeFromLastError();
+			break;
+		}
+		if (BytesRead == 0)
 		{
-			Success = (BytesRead == 0);
 			break;
 		}
 
 		if (write(ToFd.Fd, Buffer, BytesRead) != BytesRead)
 		{
-			Success = false;
+			Result = MakeErrorCodeFromLastError();
 			break;
 		}
 	}
 	free(Buffer);
-#endif	// ZEN_PLATFORM_WINDOWS
-
-	if (!Success)
+	if (Result)
 	{
-		ThrowLastError(fmt::format("file copy from {} to {} failed", FromPath, ToPath));
+		return Result;
 	}
+#endif	// ZEN_PLATFORM_WINDOWS
 
-	return true;
+	return {};
 }
 
 void
@@ -1453,24 +1453,13 @@ CopyTree(std::filesystem::path FromPath, std::filesystem::path ToPath, const Cop
 					ToPath = TargetPath / File;
 				}
 
-				try
-				{
-					if (zen::CopyFile(FromPath, ToPath, CopyOptions))
-					{
-						++FileCount;
-						ByteCount += FileSize;
-					}
-					else
-					{
-						throw std::runtime_error("CopyFile failed in an unexpected way");
-					}
-				}
-				catch (const std::exception& Ex)
+				if (std::error_code CopyEc = zen::CopyFile(FromPath, ToPath, CopyOptions); CopyEc)
 				{
 					++FailedFileCount;
-
-					throw std::runtime_error(fmt::format("failed to copy '{}' to '{}': '{}'", FromPath, ToPath, Ex.what()));
+					throw std::system_error(CopyEc, fmt::format("failed to copy '{}' to '{}'", FromPath, ToPath));
 				}
+				++FileCount;
+				ByteCount += FileSize;
 			}
 		}
 
@@ -1683,17 +1672,17 @@ WriteFile(std::filesystem::path Path, CompositeBuffer InData)
 	WriteFile(Path, DataPtrs.data(), DataPtrs.size());
 }
 
-bool
+std::error_code
 MoveToFile(std::filesystem::path Path, IoBuffer Data)
 {
 	if (!Data.IsWholeFile())
 	{
-		return false;
+		return std::make_error_code(std::errc::invalid_argument);
 	}
 	IoBufferFileReference FileRef;
 	if (!Data.GetFileReference(/* out */ FileRef))
 	{
-		return false;
+		return std::make_error_code(std::errc::invalid_argument);
 	}
 
 #if ZEN_PLATFORM_WINDOWS
@@ -1709,15 +1698,16 @@ MoveToFile(std::filesystem::path Path, IoBuffer Data)
 	RenameInfo->FileName[FileName.size()] = 0;
 
 	// Try to move file into place
-	BOOL Success = SetFileInformationByHandle(ChunkFileHandle, FileRenameInfo, RenameInfo, BufferSize);
+	BOOL  Success	= SetFileInformationByHandle(ChunkFileHandle, FileRenameInfo, RenameInfo, BufferSize);
+	DWORD LastError = Success ? ERROR_SUCCESS : GetLastError();
 
 	if (!Success)
 	{
-		DWORD LastError = GetLastError();
 		if (LastError == ERROR_PATH_NOT_FOUND)
 		{
 			zen::CreateDirectories(Path.parent_path());
-			Success = SetFileInformationByHandle(ChunkFileHandle, FileRenameInfo, RenameInfo, BufferSize);
+			Success	  = SetFileInformationByHandle(ChunkFileHandle, FileRenameInfo, RenameInfo, BufferSize);
+			LastError = Success ? ERROR_SUCCESS : GetLastError();
 		}
 		if (!Success && (LastError == ERROR_ACCESS_DENIED))
 		{
@@ -1735,23 +1725,29 @@ MoveToFile(std::filesystem::path Path, IoBuffer Data)
 					if (LastError == ERROR_PATH_NOT_FOUND)
 					{
 						zen::CreateDirectories(Path.parent_path());
-						Success = ::MoveFile(NativeSourcePath, NativeTargetPath);
+						Success	  = ::MoveFile(NativeSourcePath, NativeTargetPath);
+						LastError = Success ? ERROR_SUCCESS : GetLastError();
 					}
 				}
 			}
+			else
+			{
+				Memory::Free(RenameInfo);
+				return Ec;
+			}
 		}
 	}
 	Memory::Free(RenameInfo);
 	if (!Success)
 	{
-		return false;
+		return MakeErrorCode(LastError);
 	}
 #elif ZEN_PLATFORM_LINUX || ZEN_PLATFORM_MAC
 	std::error_code		  Ec;
 	std::filesystem::path SourcePath = PathFromHandle(FileRef.FileHandle, Ec);
 	if (Ec)
 	{
-		return false;
+		return Ec;
 	}
 	int Ret = rename(SourcePath.c_str(), Path.c_str());
 	if (Ret < 0)
@@ -1765,11 +1761,11 @@ MoveToFile(std::filesystem::path Path, IoBuffer Data)
 	}
 	if (Ret < 0)
 	{
-		return false;
+		return MakeErrorCodeFromLastError();
 	}
 #endif	// ZEN_PLATFORM_LINUX || ZEN_PLATFORM_MAC
 	Data.SetDeleteOnClose(false);
-	return true;
+	return {};
 }
 
 IoBuffer
@@ -1898,7 +1894,7 @@ ScanFile(void*												  NativeHandle,
 	}
 }
 
-bool
+std::error_code
 ScanFile(std::filesystem::path Path, const uint64_t ChunkSize, std::function<void(const void* Data, size_t Size)>&& ProcessFunc)
 {
 #if ZEN_PLATFORM_WINDOWS
@@ -1912,7 +1908,7 @@ ScanFile(std::filesystem::path Path, const uint64_t ChunkSize, std::function<voi
 	if (FromFile == INVALID_HANDLE_VALUE)
 	{
 		FromFile.Detach();
-		return false;
+		return MakeErrorCodeFromLastError();
 	}
 
 	std::vector<uint8_t> ReadBuffer(ChunkSize);
@@ -1924,7 +1920,7 @@ ScanFile(std::filesystem::path Path, const uint64_t ChunkSize, std::function<voi
 
 		if (!Success)
 		{
-			throw std::system_error(std::error_code(::GetLastError(), std::system_category()), "file scan failed");
+			return MakeErrorCodeFromLastError();
 		}
 
 		if (dwBytesRead == 0)
@@ -1936,10 +1932,10 @@ ScanFile(std::filesystem::path Path, const uint64_t ChunkSize, std::function<voi
 	ScopedFd InFd(open(Path.c_str(), O_RDONLY | O_CLOEXEC));
 	if (!InFd)
 	{
-		return false;
+		return MakeErrorCodeFromLastError();
 	}
 
-	bool Success = true;
+	std::error_code Result;
 
 	void* Buffer = malloc(ChunkSize);
 	while (true)
@@ -1947,7 +1943,7 @@ ScanFile(std::filesystem::path Path, const uint64_t ChunkSize, std::function<voi
 		int BytesRead = read(InFd.Fd, Buffer, ChunkSize);
 		if (BytesRead < 0)
 		{
-			Success = false;
+			Result = MakeErrorCodeFromLastError();
 			break;
 		}
 
@@ -1961,13 +1957,13 @@ ScanFile(std::filesystem::path Path, const uint64_t ChunkSize, std::function<voi
 
 	free(Buffer);
 
-	if (!Success)
+	if (Result)
 	{
-		ThrowLastError("file scan failed");
+		return Result;
 	}
 #endif	// ZEN_PLATFORM_WINDOWS
 
-	return true;
+	return {};
 }
 
 void
@@ -2564,7 +2560,7 @@ GetModificationTickFromPath(const std::filesystem::path& Filename)
 #endif
 }
 
-bool
+std::error_code
 TryGetFileProperties(const std::filesystem::path& Path,
 					 uint64_t&					  OutSize,
 					 uint64_t&					  OutModificationTick,
@@ -2582,31 +2578,31 @@ TryGetFileProperties(const std::filesystem::path& Path,
 								   nullptr);
 		if (Handle == INVALID_HANDLE_VALUE)
 		{
-			return false;
+			return MakeErrorCodeFromLastError();
 		}
 		auto _ = MakeGuard([Handle]() { CloseHandle(Handle); });
 
 		BY_HANDLE_FILE_INFORMATION Bhfh = {};
 		if (!GetFileInformationByHandle(Handle, &Bhfh))
 		{
-			return false;
+			return MakeErrorCodeFromLastError();
 		}
 		OutSize					  = uint64_t(Bhfh.nFileSizeHigh) << 32 | Bhfh.nFileSizeLow;
 		OutModificationTick		  = ((uint64_t(Bhfh.ftLastWriteTime.dwHighDateTime) << 32) | Bhfh.ftLastWriteTime.dwLowDateTime);
 		OutNativeModeOrAttributes = Bhfh.dwFileAttributes;
-		return true;
+		return {};
 	}
 #else
 	struct stat Stat;
 	int			err = stat(Path.native().c_str(), &Stat);
 	if (err)
 	{
-		return false;
+		return MakeErrorCodeFromLastError();
 	}
 	OutModificationTick		  = StatMtime100Ns(Stat);
 	OutSize					  = size_t(Stat.st_size);
 	OutNativeModeOrAttributes = (uint32_t)Stat.st_mode;
-	return true;
+	return {};
 #endif
 }
 
@@ -2723,10 +2719,10 @@ MaximizeOpenFileCount()
 #endif
 }
 
-bool
+std::error_code
 PrepareFileForScatteredWrite(void* FileHandle, uint64_t FinalSize)
 {
-	bool Result = true;
+	std::error_code Result;
 #if ZEN_PLATFORM_WINDOWS
 
 	BY_HANDLE_FILE_INFORMATION Information;
@@ -2738,9 +2734,9 @@ PrepareFileForScatteredWrite(void* FileHandle, uint64_t FinalSize)
 			BOOL  Ok = DeviceIoControl(FileHandle, FSCTL_SET_SPARSE, nullptr, 0, nullptr, 0, &_, nullptr);
 			if (!Ok)
 			{
+				Result = MakeErrorCodeFromLastError();
 				std::error_code DummyEc;
 				ZEN_DEBUG("Unable to set sparse mode for file '{}'", PathFromHandle(FileHandle, DummyEc));
-				Result = false;
 			}
 		}
 	}
@@ -2749,9 +2745,9 @@ PrepareFileForScatteredWrite(void* FileHandle, uint64_t FinalSize)
 	AllocationInfo.AllocationSize.QuadPart = LONGLONG(FinalSize);
 	if (!SetFileInformationByHandle(FileHandle, FileAllocationInfo, &AllocationInfo, DWORD(sizeof(AllocationInfo))))
 	{
+		Result = MakeErrorCodeFromLastError();
 		std::error_code DummyEc;
 		ZEN_DEBUG("Unable to set file allocation size to {} for file '{}'", FinalSize, PathFromHandle(FileHandle, DummyEc));
-		Result = false;
 	}
 
 #else	// ZEN_PLATFORM_WINDOWS
@@ -3713,10 +3709,11 @@ TEST_CASE("filesystem")
 	// Scan/read file
 	FileContents		 BinRead = ReadFile(BinPath);
 	std::vector<uint8_t> BinScan;
-	ScanFile(BinPath, 16 << 10, [&](const void* Data, size_t Size) {
-		const auto* Ptr = (uint8_t*)Data;
-		BinScan.insert(BinScan.end(), Ptr, Ptr + Size);
-	});
+	std::error_code		 ScanEc = ScanFile(BinPath, 16 << 10, [&](const void* Data, size_t Size) {
+		 const auto* Ptr = (uint8_t*)Data;
+		 BinScan.insert(BinScan.end(), Ptr, Ptr + Size);
+	 });
+	CHECK(!ScanEc);
 	CHECK_EQ(BinRead.Data.size(), 1);
 	CHECK_EQ(BinScan.size(), BinRead.Data[0].GetSize());
 }
@@ -3946,9 +3943,9 @@ TEST_CASE("TryCloneFile")
 		WriteFile(SrcPath, IoBuffer(IoBuffer::Wrap, Content, sizeof(Content)));
 		CHECK(IsFile(SrcPath));
 
-		bool Cloned = TryCloneFile(SrcPath, DstPath);
+		std::error_code CloneEc = TryCloneFile(SrcPath, DstPath);
 
-		if (Cloned)
+		if (!CloneEc)
 		{
 			CHECK(IsFile(DstPath));
 			CHECK_EQ(FileSizeFromPath(DstPath), sizeof(Content));
@@ -3961,7 +3958,7 @@ TEST_CASE("TryCloneFile")
 		else
 		{
 			// Clone not supported on this filesystem - that's okay, just verify it didn't leave debris
-			ZEN_INFO("TryCloneFile not supported on this filesystem, skipping content check");
+			ZEN_INFO("TryCloneFile not supported on this filesystem ({}), skipping content check", CloneEc.message());
 		}
 	}
 
@@ -3975,9 +3972,9 @@ TEST_CASE("TryCloneFile")
 		WriteFile(DstPath, IoBuffer(IoBuffer::Wrap, OldContent, sizeof(OldContent)));
 		WriteFile(SrcPath, IoBuffer(IoBuffer::Wrap, NewContent, sizeof(NewContent)));
 
-		bool Cloned = TryCloneFile(SrcPath, DstPath);
+		std::error_code CloneEc = TryCloneFile(SrcPath, DstPath);
 
-		if (Cloned)
+		if (!CloneEc)
 		{
 			CHECK_EQ(FileSizeFromPath(DstPath), sizeof(NewContent));
 
@@ -3992,7 +3989,7 @@ TEST_CASE("TryCloneFile")
 		std::filesystem::path SrcPath = TestBaseDir / "no_such_file.bin";
 		std::filesystem::path DstPath = TestBaseDir / "dst_nosrc.bin";
 
-		CHECK_FALSE(TryCloneFile(SrcPath, DstPath));
+		CHECK(TryCloneFile(SrcPath, DstPath));
 		CHECK_FALSE(IsFile(DstPath));
 	}
 
@@ -4014,8 +4011,8 @@ TEST_CASE("CopyFile.Clone")
 
 		CopyFileOptions Options;
 		Options.EnableClone = true;
-		bool Success		= CopyFile(SrcPath, DstPath, Options);
-		CHECK(Success);
+		std::error_code Ec	= CopyFile(SrcPath, DstPath, Options);
+		CHECK(!Ec);
 		CHECK(IsFile(DstPath));
 		CHECK_EQ(FileSizeFromPath(DstPath), sizeof(Content));
 
@@ -4030,8 +4027,8 @@ TEST_CASE("CopyFile.Clone")
 
 		CopyFileOptions Options;
 		Options.EnableClone = false;
-		bool Success		= CopyFile(SrcPath, DstPath, Options);
-		CHECK(Success);
+		std::error_code Ec	= CopyFile(SrcPath, DstPath, Options);
+		CHECK(!Ec);
 		CHECK(IsFile(DstPath));
 		CHECK_EQ(FileSizeFromPath(DstPath), sizeof(Content));