Use thread-safe strerror_r() to get error message
#8087
Conversation
|
@jay-zhuang has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
|
the vs2019-cxx20 build failure is unrelated, should be fixed by: #8090 |
| @@ -141,4 +141,6 @@ bool SerializeIntVector(const std::vector<int>& vec, std::string* value); | |||
|
|
|||
| extern const std::string kNullptrString; | |||
|
|
|||
| extern std::string errnoStr(int err); | |||
There was a problem hiding this comment.
Please add a comment as to what this function is used for.
port/win/env_win.cc
Outdated
| fprintf(stderr, "pthread %s: %s\n", label, | ||
| errnoStr(result.value()).c_str()); |
There was a problem hiding this comment.
Not that it is related to this fix, but shouldn't this be WinThread, not pthread?
| @@ -61,7 +62,8 @@ typedef std::unique_ptr<void, decltype(FindCloseFunc)> UniqueFindClosePtr; | |||
|
|
|||
| void WinthreadCall(const char* label, std::error_code result) { | |||
| if (0 != result.value()) { | |||
| fprintf(stderr, "pthread %s: %s\n", label, strerror(result.value())); | |||
There was a problem hiding this comment.
Is there a reason to go through errnoStr here rather than calling the Windows function directly? Just curious if the redirection is worth it and does not obfuscate the code a bit.
There was a problem hiding this comment.
yeah, either way works, but as the new function also needs to allocate a buffer, it might be simpler just calling a centralized function errnoStr().
|
@jay-zhuang has updated the pull request. You must reimport the pull request before landing. |
|
@jay-zhuang has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
|
@jay-zhuang has updated the pull request. You must reimport the pull request before landing. |
|
@jay-zhuang has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator. |
|
@jay-zhuang merged this pull request in 45c65d6. |
Summary: `strerror()` is not thread-safe, using `strerror_r()` instead. The API could be different on the different platforms, used the code from https://github.com/facebook/folly/blob/0deef031cb8aab76dc7e736f8b7c22d701d5f36b/folly/String.cpp#L457 Pull Request resolved: facebook/rocksdb#8087 Reviewed By: mrambacher Differential Revision: D27267151 Pulled By: jay-zhuang fbshipit-source-id: 4b8856d1ec069d5f239b764750682c56e5be9ddb Signed-off-by: Changlong Chen <levisonchen@live.cn>
Summary: `strerror()` is not thread-safe, using `strerror_r()` instead. The API could be different on the different platforms, used the code from https://github.com/facebook/folly/blob/0deef031cb8aab76dc7e736f8b7c22d701d5f36b/folly/String.cpp#L457 Pull Request resolved: facebook/rocksdb#8087 Reviewed By: mrambacher Differential Revision: D27267151 Pulled By: jay-zhuang fbshipit-source-id: 4b8856d1ec069d5f239b764750682c56e5be9ddb Signed-off-by: Changlong Chen <levisonchen@live.cn>
strerror()is not thread-safe, usingstrerror_r()instead. The API could be different on the different platforms, used the code from https://github.com/facebook/folly/blob/0deef031cb8aab76dc7e736f8b7c22d701d5f36b/folly/String.cpp#L457