clang(windows) getenv deprecation warn fix #900
Conversation
|
Thanks! |
| #else | ||
| size_t len = 0; | ||
| char buf[1024]; | ||
| bool ok = ::getenv_s(&len, buf, sizeof(buf), field) == 0; |
There was a problem hiding this comment.
getenv_s is a C11 function. Is it guaranteed to be defined in C++11?
| std::string getenv_safe(const char *field) { | ||
| #if defined(_MSC_VER) | ||
| #if defined(__cplusplus_winrt) | ||
| return std::string{}; // not supported under uwp |
There was a problem hiding this comment.
Does it means FTXUI won't work under UWP? This sounds problematic.
There was a problem hiding this comment.
To be honest, I haven't tested this under UWP, but I can assume that UWP applications don't have access to environment variables.
| return ok ? buf : std::string{}; | ||
| #endif | ||
| #else // revert to getenv | ||
| char *buf = ::getenv(field); // NOLINT(*-mt-unsafe) |
There was a problem hiding this comment.
I believe the previous implementation was meant to fix this *-mt-unsafe warning. We probably want to revert it back (e.g. do not store the output in a non temporary object)
| bool Contains(const std::string& s, const char* key) { | ||
| return s.find(key) != std::string::npos; | ||
| } | ||
|
|
||
| // https://github.com/gabime/spdlog/blob/885b5473e291833b148eeac3b7ce227e582cd88b/include/spdlog/details/os-inl.h#L566 |
There was a problem hiding this comment.
I don't see any warning on my bots:
https://github.com/ArthurSonzogni/FTXUI/actions/runs/9695462958/job/26755276508
What configs should I add to reproduce?
Are you using the build config the FTXUI project define (e.g. are you using CMake at the end)
There was a problem hiding this comment.
Oh, I'm sorry. This is not msvc, but clang for windows.
cmake -Bbuild-clang -DCMAKE_CXX_COMPILER=clang++.exe -DCMAKE_C_COMPILER=clang.exe -GNinjaC:/Users/Alex/Desktop/FTXUI/src/ftxui/screen/terminal.cpp:64:37: warning: 'getenv' is deprecated: This function or variable may be unsafe. Consider using _dupenv_s instead. To disable deprecation, use _CRT_SECURE_NO_WARNINGS. See online help for details. [-Wdeprecated-declarations]
64 | std::string COLORTERM = Safe(std::getenv("COLORTERM")); // NOLINT
There was a problem hiding this comment.
@ArthurSonzogni see workflow run on my fork
https://github.com/alexv-ds/FTXUI/actions/runs/10021288139/job/27699782082#step:6:72
alexv-ds
changed the title
ArthurSonzogni
force-pushed
the
main
branch
5 times, most recently
from
f353474
to
c5357ac
Compare
getenv in clang(windows) is deprecated
based on spdlog