[libc-commits] [PATCH] D135322: [libc] add strsignal and refactor message mapping

[Michael Jones via Phabricator via libc-commits]

michaelrj added inline comments.


================
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);
----------------
sivachandra wrote:
> 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();
> }
> ```
For comments on the `strerror_r` design you should comment on this patch instead: https://reviews.llvm.org/D135227 but overall I think the idea makes sense.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D135322/new/

https://reviews.llvm.org/D135322