aaron.ballman added inline comments.
================
Comment at: clang/lib/Driver/Driver.cpp:5427
+ ? A->getValue()
+ :
std::getenv("CLANG_CRASH_DIAGNOSTICS_DIR");
+ if (CrashDirectory) {
----------------
mizvekov wrote:
> aaron.ballman wrote:
> > This should use `llvm::sys::Process::GetEnv()` so it properly handles text
> > encodings. CC @tahonermann @cor3ntin for text encoding awareness.
> Okay, makes sense. But be aware that we have multiple places in the llvm
> project where we are using std::getenv to retrieve a filesystem path
> variable. There is another one within clang itself as well
> (`CLANG_MODULE_CACHE_PATH`, which is where I took inspiration from).
Yeah, I noticed the same thing when I was looking around. Every one of those is
a bug! It likely works fine for Linux where UTF-8 is common, but they will all
fail on Windows where UTF-16 is used for the OS but `::getenv()` uses the C
locale. Any sort of non-ASCII characters in the file path are likely not going
to do the right thing in that case.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D133082/new/
https://reviews.llvm.org/D133082
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
