[libcxx-commits] [PATCH] D158930: [libc++][print] Fixes error reporting.

[Ryan Prichard via Phabricator via libcxx-commits]

rprichard added subscribers: enh, rprichard.
rprichard added inline comments.


================
Comment at: libcxx/include/print:218-219
 
+  // On POSIX errno is set by fwrite. This is not required by the C standard.
+  errno         = 0;
   size_t __size = fwrite(__str.data(), 1, __str.size(), __stream);
----------------
ldionne wrote:
> I am a bit concerned with the direct usage of `errno` here. What about thread safety?
> 
> IMO the previous approach with `ferror` made more sense, we just shouldn't be using the return value of `ferror` as more than a zero/non-zero indicator. WDYT?
POSIX defines errno as thread-local, and a non-thread-local errno would be mostly unusable:
 - https://stackoverflow.com/a/1694170
 - https://pubs.opengroup.org/onlinepubs/9699919799/xrat/V4_xsh_chap02.html#tag_22_02_03

Maybe it'd make sense to save/restore errno in case the caller expects its value to be unaffected by a C++ API. Not sure. (e.g. Bionic has a ErrnoRestorer class that it uses to avoid leaking errno modifications to callers.)

I don't really have an opinion right now on feof and ferror usage, except that the Bionic-vs-glibc difference makes me wonder if ferror would give inconsistent results (https://reviews.llvm.org/D150044#4597254).



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D158930