[libc-commits] [PATCH] D134074: [libc] add strerror

[Michael Jones via Phabricator via libc-commits]

michaelrj added inline comments.


================
Comment at: libc/src/__support/error_to_string.h:1
+//===-- Definition of a class for mapping errors to strings -----*- C++ -*-===//
+//
----------------
sivachandra wrote:
> Almost everything in this file should live in a `.cpp` file. The only thing declared in the .h file should something like:
> 
> ```
> cpp::string_view error_to_string(int error);
> ```
> 
> IMO, that single function should take care of unknown error numbers also. Do you have a use case where `max_buff_size` and `get_unknown_str` are required?
> 
> A more fundamental question - is there any reason why this should live in `__support` at all?
I did make a .cpp file to contain the functions for the class, but was running into compile errors where the linker couldn't find the implementations, so I moved it into the header for now.

As for the other parts:
The reason `max_buff_size` and `get_unknown_str` exist is that when an error without a specified string mapping is passed, we need to generate a new string that says `Unknown error [number]`. Since this string will be passed back to the caller code, it can't be on the stack, so we need a buffer to hold it. In `strerror.cpp` I define a global buffer of size `max_buff_size` to be that buffer. This is specifically allowed by the standard: "The strerror function returns a pointer to the string, the contents of which are localespecific. The array pointed to shall not be modified by the program, but may be overwritten by a subsequent call to the strerror function."

I placed this in support because both `strerror` which is in strings.h and `perror` which is in stdio.h use the same functionality. I can move it into strings, but generally I try to avoid including files between headers when possible.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134074