cameron314 added a comment. I'll run clang-format and submit a new patch. Thanks for fixing that duplicate symbol with the signal stuff!
================ Comment at: source/Host/common/FileSpec.cpp:94-115 @@ -93,2 +93,24 @@ + { +#ifdef _WIN32 + std::wstring wresolved_path; + if (!llvm::ConvertUTF8toWide(resolved_path, wresolved_path)) + return false; + int stat_result; +#ifdef _USE_32BIT_TIME_T + struct _stat32 file_stats; + stat_result = ::_wstat32(wresolved_path.c_str(), &file_stats); +#else + struct _stat64i32 file_stats; + stat_result = ::_wstat64i32(wresolved_path.c_str(), &file_stats); +#endif + if (stat_result == 0) + { + static_assert(sizeof(struct stat) == sizeof(file_stats), "stat and _stat32/_stat64i32 must have the same layout"); + *stats_ptr = *reinterpret_cast<struct stat *>(&file_stats); + } + return stat_result == 0; +#else return ::stat (resolved_path, stats_ptr) == 0; +#endif + } return false; ---------------- zturner wrote: > Here we are doing some stuff with `_USE_32BIT_TIME_T`, but we reproduce > almost this exact same logic in `File::GetPermissions()` except without the > special case for `_USE_32BIT_TIME_T`. I think we should introduce a function > called `FileSystem::Stat()` in very much the same way that you've done > `FileSystem::Fopen()`. And then we put the implementation there so that we > can have it in one place and make sure it's always going to be consistent and > correct. Yeah, I agree this should really go in `FileSystem`. ================ Comment at: tools/lldb-mi/MIUtilFileStd.cpp:82-95 @@ -79,9 +81,16 @@ #if !defined(_MSC_VER) // Open with 'write' and 'binary' mode m_pFileHandle = ::fopen(vFileNamePath.c_str(), "wb"); #else // Open a file with exclusive write and shared read permissions - m_pFileHandle = ::_fsopen(vFileNamePath.c_str(), "wb", _SH_DENYWR); + std::wstring path; + if (llvm::ConvertUTF8toWide(vFileNamePath.c_str(), path)) + m_pFileHandle = ::_wfsopen(path.c_str(), L"wb", _SH_DENYWR); + else + { + errno = EINVAL; + m_pFileHandle = nullptr; + } #endif // !defined( _MSC_VER ) ---------------- zturner wrote: > Should we be using `FileSystem::Fopen()` here? Since this is MSVC specific code already, I think it's OK. The `_SH_DENYWR` seems important here. ================ Comment at: tools/lldb-mi/MIUtilFileStd.cpp:234-241 @@ -224,3 +233,10 @@ FILE *pTmp = nullptr; +#ifdef _WIN32 + std::wstring path; + if (!llvm::ConvertUTF8toWide(vFileNamePath.c_str(), path)) + return false; + pTmp = ::_wfopen(path.c_str(), L"wb"); +#else pTmp = ::fopen(vFileNamePath.c_str(), "wb"); +#endif if (pTmp != nullptr) ---------------- zturner wrote: > Same here. `FileSystem::Fopen()` Absolutely. I missed this one. http://reviews.llvm.org/D17107 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits