From 7cfd37a52c08d0bbf4b1d89b7cfa9f8b6660b9cf Mon Sep 17 00:00:00 2001 From: DeltaDizzy Date: Thu, 30 Jan 2025 11:40:27 -0500 Subject: [PATCH 1/4] [wpiutil] use reinterpret_cast for kInvalidFile on win32 --- .../native/cpp/DataLogBackgroundWriter.cpp | 14 +++++----- wpiutil/src/main/native/cpp/fs.cpp | 26 +++++++++---------- wpiutil/src/main/native/include/wpi/fs.h | 14 +++++----- 3 files changed, 26 insertions(+), 28 deletions(-) diff --git a/wpiutil/src/main/native/cpp/DataLogBackgroundWriter.cpp b/wpiutil/src/main/native/cpp/DataLogBackgroundWriter.cpp index 9b7a52c9d06..c8028af1289 100644 --- a/wpiutil/src/main/native/cpp/DataLogBackgroundWriter.cpp +++ b/wpiutil/src/main/native/cpp/DataLogBackgroundWriter.cpp @@ -183,9 +183,9 @@ struct DataLogBackgroundWriter::WriterThreadState { ~WriterThreadState() { Close(); } void Close() { - if (f != fs::kInvalidFile) { + if (f != WPI_kInvalidFile) { fs::CloseFile(f); - f = fs::kInvalidFile; + f = WPI_kInvalidFile; } } @@ -207,7 +207,7 @@ struct DataLogBackgroundWriter::WriterThreadState { std::string baseFilename; std::string filename; fs::path path; - fs::file_t f = fs::kInvalidFile; + fs::file_t f = WPI_kInvalidFile; uintmax_t freeSpace = UINTMAX_MAX; int segmentCount = 1; }; @@ -264,7 +264,7 @@ void DataLogBackgroundWriter::StartLogFile(WriterThreadState& state) { } } - if (state.f == fs::kInvalidFile) { + if (state.f == WPI_kInvalidFile) { WPI_ERROR(m_msglog, "Could not open log file, no log being saved"); } else { WPI_INFO(m_msglog, "Logging to '{}' ({} free space)", state.path.string(), @@ -273,7 +273,7 @@ void DataLogBackgroundWriter::StartLogFile(WriterThreadState& state) { } // start file - if (state.f != fs::kInvalidFile) { + if (state.f != WPI_kInvalidFile) { StartFile(); } } @@ -347,7 +347,7 @@ void DataLogBackgroundWriter::WriterThreadMain(std::string_view dir) { written = 0; } - if (!m_newFilename.empty() && state.f != fs::kInvalidFile) { + if (!m_newFilename.empty() && state.f != WPI_kInvalidFile) { auto newFilename = std::move(m_newFilename); m_newFilename.clear(); // rename @@ -374,7 +374,7 @@ void DataLogBackgroundWriter::WriterThreadMain(std::string_view dir) { continue; } - if (state.f != fs::kInvalidFile && !blocked) { + if (state.f != WPI_kInvalidFile && !blocked) { lock.unlock(); // update free space every 10 flushes (in case other things are writing) diff --git a/wpiutil/src/main/native/cpp/fs.cpp b/wpiutil/src/main/native/cpp/fs.cpp index ed6829792c8..e62350c3694 100644 --- a/wpiutil/src/main/native/cpp/fs.cpp +++ b/wpiutil/src/main/native/cpp/fs.cpp @@ -59,8 +59,6 @@ namespace fs { #pragma warning(disable : 4244 4267 4146) #endif -const file_t kInvalidFile = INVALID_HANDLE_VALUE; - static DWORD nativeDisposition(CreationDisposition Disp, OpenFlags Flags) { switch (Disp) { case CD_CreateAlways: @@ -107,12 +105,12 @@ static file_t openFileInternal(const path& Path, std::error_code& EC, // This only runs if we failed to open the file, so there is probably // no performances issues. if (LastError != ERROR_ACCESS_DENIED) { - return kInvalidFile; + return WPI_kInvalidFile; } if (is_directory(Path)) { EC = std::make_error_code(std::errc::is_a_directory); } - return kInvalidFile; + return WPI_kInvalidFile; } EC = std::error_code(); return H; @@ -156,14 +154,14 @@ file_t OpenFile(const path& Path, std::error_code& EC, CreationDisposition Disp, DWORD LastError = ::GetLastError(); ::CloseHandle(Result); EC = wpi::mapWindowsError(LastError); - return kInvalidFile; + return WPI_kInvalidFile; } } if (Flags & OF_Delete) { if ((EC = setDeleteDisposition(Result, true))) { ::CloseHandle(Result); - return kInvalidFile; + return WPI_kInvalidFile; } } return Result; @@ -174,7 +172,7 @@ file_t OpenFileForRead(const path& Path, std::error_code& EC, OpenFlags Flags) { } int FileToFd(file_t& F, std::error_code& EC, OpenFlags Flags) { - if (F == kInvalidFile) { + if (F == WPI_kInvalidFile) { EC = wpi::mapWindowsError(ERROR_INVALID_HANDLE); return -1; } @@ -196,18 +194,18 @@ int FileToFd(file_t& F, std::error_code& EC, OpenFlags Flags) { } EC = std::error_code(); - F = kInvalidFile; + F = WPI_kInvalidFile; return ResultFD; } void CloseFile(file_t& F) { ::CloseHandle(F); - F = kInvalidFile; + F = WPI_kInvalidFile; } #else // _WIN32 -const file_t kInvalidFile = -1; +const file_t WPI_kInvalidFile = -1; static int nativeOpenFlags(CreationDisposition Disp, OpenFlags Flags, FileAccess Access) { @@ -248,14 +246,14 @@ static int nativeOpenFlags(CreationDisposition Disp, OpenFlags Flags, file_t OpenFile(const path& Path, std::error_code& EC, CreationDisposition Disp, FileAccess Access, OpenFlags Flags, unsigned Mode) { int OpenFlags = nativeOpenFlags(Disp, Flags, Access); - file_t ResultFD = kInvalidFile; + file_t ResultFD = WPI_kInvalidFile; // Call ::open in a lambda to avoid overload resolution in RetryAfterSignal // when open is overloaded, such as in Bionic. auto Open = [&]() { return ::open(Path.c_str(), OpenFlags, Mode); }; if ((ResultFD = wpi::sys::RetryAfterSignal(-1, Open)) < 0) { EC = std::error_code(errno, std::generic_category()); - return kInvalidFile; + return WPI_kInvalidFile; } #ifndef O_CLOEXEC if (!(Flags & OF_ChildInherit)) { @@ -274,14 +272,14 @@ file_t OpenFileForRead(const path& Path, std::error_code& EC, OpenFlags Flags) { int FileToFd(file_t& F, std::error_code& EC, OpenFlags Flags) { int fd = F; - F = kInvalidFile; + F = WPI_kInvalidFile; EC = std::error_code(); return fd; } void CloseFile(file_t& F) { ::close(F); - F = kInvalidFile; + F = WPI_kInvalidFile; } #endif // _WIN32 diff --git a/wpiutil/src/main/native/include/wpi/fs.h b/wpiutil/src/main/native/include/wpi/fs.h index 47de1d55660..3505700d7ce 100644 --- a/wpiutil/src/main/native/include/wpi/fs.h +++ b/wpiutil/src/main/native/include/wpi/fs.h @@ -28,12 +28,12 @@ using fstream = std::fstream; #if defined(_WIN32) // A Win32 HANDLE is a typedef of void* using file_t = void*; +#define WPI_kInvalidFile reinterpret_cast(-1) #else using file_t = int; +#define WPI_kInvalidFile -1 #endif -extern const file_t kInvalidFile; - enum CreationDisposition : unsigned { /// CD_CreateAlways - When opening a file: /// * If it already exists, truncate it. @@ -139,7 +139,7 @@ file_t OpenFile(const path& Path, std::error_code& EC, CreationDisposition Disp, * opened in, for example, read-write or in write-only mode. * @param Mode The access permissions of the file, represented in octal. * @returns a platform-specific file descriptor if \a Name has been opened, - * otherwise kInvalidFile. + * otherwise WPI_kInvalidFile. */ inline file_t OpenFileForWrite(const path& Path, std::error_code& EC, CreationDisposition Disp, OpenFlags Flags, @@ -162,7 +162,7 @@ inline file_t OpenFileForWrite(const path& Path, std::error_code& EC, * opened in, for example, read-write or in write-only mode. * @param Mode The access permissions of the file, represented in octal. * @return a platform-specific file descriptor if \a Name has been opened, - * otherwise kInvalidFile. + * otherwise WPI_kInvalidFile. */ inline file_t OpenFileForReadWrite(const path& Path, std::error_code& EC, CreationDisposition Disp, OpenFlags Flags, @@ -181,7 +181,7 @@ inline file_t OpenFileForReadWrite(const path& Path, std::error_code& EC, * @param EC Error code output, set to non-zero on error * @param Flags Additional flags * @return a platform-specific file descriptor if \a Name has been opened, - * otherwise kInvalidFile. + * otherwise WPI_kInvalidFile. */ file_t OpenFileForRead(const path& Path, std::error_code& EC, OpenFlags Flags = OF_None); @@ -191,7 +191,7 @@ file_t OpenFileForRead(const path& Path, std::error_code& EC, * must be closed with ::close() instead of CloseFile(). * * @param F On input, this is the file to convert to a file descriptor. - * On output, the file is set to kInvalidFile. + * On output, the file is set to WPI_kInvalidFile. * @param EC Error code output, set to non-zero on error * @param Flags Flags passed to the OpenFile function that created file_t * @return file descriptor, or -1 on error @@ -202,7 +202,7 @@ int FileToFd(file_t& F, std::error_code& EC, OpenFlags Flags); * Closes the file object. * * @param F On input, this is the file to close. On output, the file is - * set to kInvalidFile. + * set to WPI_kInvalidFile. */ void CloseFile(file_t& F); From d2207760e47369eb5c12bc6ad2a0e5b9e13fc32e Mon Sep 17 00:00:00 2001 From: DeltaDizzy Date: Thu, 30 Jan 2025 11:49:46 -0500 Subject: [PATCH 2/4] remove extra WIN32 define --- wpiutil/src/main/native/cpp/fs.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/wpiutil/src/main/native/cpp/fs.cpp b/wpiutil/src/main/native/cpp/fs.cpp index e62350c3694..82bd3857eed 100644 --- a/wpiutil/src/main/native/cpp/fs.cpp +++ b/wpiutil/src/main/native/cpp/fs.cpp @@ -205,8 +205,6 @@ void CloseFile(file_t& F) { #else // _WIN32 -const file_t WPI_kInvalidFile = -1; - static int nativeOpenFlags(CreationDisposition Disp, OpenFlags Flags, FileAccess Access) { int Result = 0; From 678ef578b3c84b4127fc1e77ee09422044544226 Mon Sep 17 00:00:00 2001 From: DeltaDizzy Date: Thu, 30 Jan 2025 13:02:59 -0500 Subject: [PATCH 3/4] use file_t in cast --- wpiutil/src/main/native/include/wpi/fs.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/wpiutil/src/main/native/include/wpi/fs.h b/wpiutil/src/main/native/include/wpi/fs.h index 3505700d7ce..4e762c34e46 100644 --- a/wpiutil/src/main/native/include/wpi/fs.h +++ b/wpiutil/src/main/native/include/wpi/fs.h @@ -28,7 +28,7 @@ using fstream = std::fstream; #if defined(_WIN32) // A Win32 HANDLE is a typedef of void* using file_t = void*; -#define WPI_kInvalidFile reinterpret_cast(-1) +#define WPI_kInvalidFile reinterpret_cast(-1) #else using file_t = int; #define WPI_kInvalidFile -1 From e25a9358b79189a8e319d8532d3dc71a2a8e9615 Mon Sep 17 00:00:00 2001 From: DeltaDizzy Date: Thu, 30 Jan 2025 13:13:07 -0500 Subject: [PATCH 4/4] use fs::file_t --- wpiutil/src/main/native/include/wpi/fs.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/wpiutil/src/main/native/include/wpi/fs.h b/wpiutil/src/main/native/include/wpi/fs.h index 4e762c34e46..82116288365 100644 --- a/wpiutil/src/main/native/include/wpi/fs.h +++ b/wpiutil/src/main/native/include/wpi/fs.h @@ -28,7 +28,7 @@ using fstream = std::fstream; #if defined(_WIN32) // A Win32 HANDLE is a typedef of void* using file_t = void*; -#define WPI_kInvalidFile reinterpret_cast(-1) +#define WPI_kInvalidFile reinterpret_cast(-1) #else using file_t = int; #define WPI_kInvalidFile -1