From ec210a87dbc4a04a102e20015ae8d0f0f973237c Mon Sep 17 00:00:00 2001 From: Dion Moult Date: Mon, 14 Sep 2026 13:04:50 +1000 Subject: [PATCH] ifcparse: use std::filesystem in guess_file_type() The stat()-based path helpers were added in 573e53ebf as a speculative workaround for #7131 ("Ugly workarounds to not depend on std::filesystem"). That issue turned out to be a hardcoded schema list missing HEADER_SECTION_SCHEMA and was fixed separately. Since the plug-in architecture landed, libIfcParse already depends on std::filesystem: schema.h exports schema_plugin_directory() returning a std::filesystem::path, and plugin.cpp uses it throughout. The build also mandates C++17. The workaround therefore no longer avoids anything and its comment is misleading. Restore the std::filesystem version, using the error_code overloads so inaccessible paths are still reported as "not there" rather than throwing, and route the path through ifcopenshell::path::from_utf8 so non-ASCII paths work on Windows, as file_reader.cpp already does. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01WcUNL8YygRrNC5KpxELMHs --- src/ifcparse/file.cpp | 59 ++++++++++--------------------------------- 1 file changed, 13 insertions(+), 46 deletions(-) diff --git a/src/ifcparse/file.cpp b/src/ifcparse/file.cpp index dc6a313880..8c805e52c2 100644 --- a/src/ifcparse/file.cpp +++ b/src/ifcparse/file.cpp @@ -1,5 +1,6 @@ #include "file.h" #include "logger.h" +#include "utils.h" #ifdef IFOPSH_WITH_ROCKSDB #include @@ -7,10 +8,10 @@ #include #endif +#include #include #include -#include -#include +#include #include /* @@ -269,58 +270,24 @@ express::base ifcopenshell::impl::in_memory_file_storage::instance_by_id(int id) ifcopenshell::file::~file() {} -namespace { - // Utility functions for path handling in order not to rely on C++17's std::filesystem -#ifdef _WIN32 -#define stat_t struct _stat - inline int stat_(const char* p, stat_t* s) { return ::_stat(p, s); } -#ifndef S_ISDIR -#define S_ISDIR(m) (((m) & _S_IFDIR) != 0) -#endif -#ifndef S_ISREG -#define S_ISREG(m) (((m) & _S_IFREG) != 0) -#endif -#else - using stat_t = struct stat; - inline int stat_(const char* p, stat_t* s) { return ::stat(p, s); } -#endif - - inline bool path_exists_(const std::string& p, stat_t* out = nullptr) { - stat_t tmp; - stat_t* s = out ? out : &tmp; - return stat_(p.c_str(), s) == 0; - } - - inline bool path_is_directory_(const stat_t& s) { return S_ISDIR(s.st_mode); } - inline bool path_is_regular_file_(const stat_t& s) { return S_ISREG(s.st_mode); } - - inline std::string path_join_(const std::string& dir, const std::string& name) { - if (dir.empty()) return name; - const char last = dir.back(); - if (last == '/' || last == '\\') return dir + name; -#ifdef _WIN32 - const char sep = '\\'; -#else - const char sep = '/'; -#endif - return dir + sep + name; - } -} // namespace - ifcopenshell::filetype ifcopenshell::guess_file_type(const std::string& fn) { - stat_t st{}; - if (!path_exists_(fn, &st)) { + namespace fs = std::filesystem; + + // The error_code overloads report inaccessible paths as "not there" + // instead of throwing, matching the previous stat()-based behaviour. + std::error_code ec; + const fs::path path(ifcopenshell::path::from_utf8(fn)); + if (!fs::exists(path, ec)) { // @todo this is just weird, but for consistency with earlier behaviour // for now the only intent for this function is to auto-detect RocksDB return FT_IFCSPF; } - if (path_is_directory_(st)) { + if (fs::is_directory(path, ec)) { // Typical RocksDB file to look for - auto currentFile = path_join_(fn, "CURRENT"); - stat_t cst{}; + const auto currentFile = path / "CURRENT"; - if (!path_exists_(currentFile, &cst) || !path_is_regular_file_(cst)) { + if (!fs::is_regular_file(currentFile, ec)) { return FT_UNKNOWN; }