[libc-commits] [PATCH] D135322: [libc] add strsignal and refactor message mapping
Siva Chandra via Phabricator via libc-commits
libc-commits at lists.llvm.org
Wed Oct 5 15:23:11 PDT 2022
sivachandra added a comment.
The new `MsgMapper` LGTM but I have comments about other parts of the design.
================
Comment at: libc/src/__support/StringUtil/error_to_string.cpp:194
+
+cpp::string_view get_error_string(int err_num, cpp::string_view buffer) {
+ auto opt_str = internal::error_mapper.get_str(err_num);
----------------
It seems to me like we should not have this overload in the `stringutil` at all because the actual libc functions need to set errno. Instead, provide a function like:
```
bool fill_unknown_error_msg(int err_num, cpp::span<char> buffer) {
...
// return true if buffer was big enough, else false.
// This will help strerror_r set errno to ERANGE based on the return value.
}
```
So, `strerror` should be implemented like this:
```
LLVM_LIBC_FUNCTION(... strerror ...) {
auto err_str = get_error_string(num);
if (num)
return const_cast<char *>(num.data());
errno = EINVAL;
// unknown_err_buffer is declared in strerror.cpp.
// In case of strerror_r, it is instantiated from the buffer argument.
fill_unknown_err_msg(num, unknown_err_buffer);
return unknown_err_buffer.data();
}
```
================
Comment at: libc/src/__support/StringUtil/message_mapper.h:23
+
+public:
+ constexpr MsgMapping(int init_num, const char *init_msg)
----------------
This is a struct so `public` is default.
================
Comment at: libc/src/__support/StringUtil/message_mapper.h:68
+ msg_offsets[num] != -1) {
+ return {const_cast<char *>(string_array + msg_offsets[num])};
+ } else {
----------------
Why is the `const_cast` required?
================
Comment at: libc/src/__support/StringUtil/signal_to_string.cpp:119
+// }
+// };
+
----------------
Delete the commented out blocks above?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D135322/new/
https://reviews.llvm.org/D135322
More information about the libc-commits
mailing list