[libc++][print] Fixes error reporting.

[Mordante]

Summary

This addresses D150044 post-commit review comments.

[ldionne]

libcxx/include/print

218–219

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?

[rprichard]

libcxx/include/print

218–219

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).

[enh]

libcxx/include/print

218–219

/me wonders whether this should all just be deleted anyway until/unless someone comes along with an actual motivating case for trying to be more specific?

this seems fine?

if (__size < __str.size()) {
  std::__throw_system_error(EIO, "failed to write formatted output");
}

POSIX explicitly states that a fwrite() that returns less than you asked it to write should set the error indicator (so by that, ferror() must always be non-zero, so there's no reason to explicitly check it).

but, yeah, if you do want to be fancy, the "save/restore errno" code sgtm:

int saved_errno = errno;
errno = 0;
size_t __size = fwrite(__str.data(), 1, __str.size(), __stream);
if (__size < __str.size()) {
  std::__throw_system_error(errno, "failed to write formatted output");
}
errno = saved_errno;

(it looks like std::system_error already does the right thing should errno still be zero because the fwrite() failure was not actually an error from the OS --- because the multiplication overflowed or whatever [not that that can happen multiplying by 1 like here, but ykwim!].)

libcxx/include/print

 template <class = void> // TODO PRINT template or availability markup fires too eagerly (http://llvm.org/PR61563).
 _LIBCPP_HIDE_FROM_ABI inline void
 __vprint_nonunicode(FILE* __stream, string_view __fmt, format_args __args, bool __write_nl) {
   _LIBCPP_ASSERT_UNCATEGORIZED(__stream, "__stream is a valid pointer to an output C stream");
   string __str = std::vformat(__fmt, __args);
   if (__write_nl)
     __str.push_back('\n');
 
+  // 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);
   if (__size < __str.size()) {
+    if (errno)
+      std::__throw_system_error(errno, "failed to write formatted output");
     if (std::feof(__stream))
       std::__throw_system_error(EIO, "EOF while writing the formatted output");
-    std::__throw_system_error(std::ferror(__stream), "failed to write formatted output");
+    std::__throw_system_error(EIO, "failed to write formatted output");
   }
 }
 
 #  ifndef _LIBCPP_HAS_NO_UNICODE
 
 // Note these helper functions are mainly used to aid testing.
 // On POSIX systems and Windows the output is no longer considered a
 // terminal when the output is redirected. Typically during testing the

libcxx/test/std/input.output/iostream.format/print.fun/print.file.pass.cpp

static void test_wide_stream() {
   assert(file);
 
   int mode = std::fwide(file, 1);
   assert(mode > 0);
 
   TEST_VALIDATE_EXCEPTION(
       std::system_error,
       [&]([[maybe_unused]] const std::system_error& e) {
-        [[maybe_unused]] std::string_view what{"failed to write formatted output"};
+        [[maybe_unused]] std::string_view what{"failed to write formatted output: Input/output error"};
         TEST_LIBCPP_REQUIRE(
             e.what() == what,
             TEST_WRITE_CONCATENATED("\nExpected exception ", what, "\nActual exception   ", e.what(), '\n'));
       },
       std::print(file, "hello"));
 }
 #endif // defined(TEST_HAS_GLIBC) && !defined(TEST_HAS_NO_WIDE_CHARACTERS)
 
 static void test_read_only() {
   FILE* file = fopen(filename.c_str(), "r");
   assert(file);
 
   TEST_VALIDATE_EXCEPTION(
       std::system_error,
       [&]([[maybe_unused]] const std::system_error& e) {
 #ifdef _AIX
         [[maybe_unused]] std::string_view what{"failed to write formatted output: Broken pipe"};
 #else
-        [[maybe_unused]] std::string_view what{"failed to write formatted output: Operation not permitted"};
+        [[maybe_unused]] std::string_view what{"failed to write formatted output: Bad file descriptor"};
 #endif
         TEST_LIBCPP_REQUIRE(
             e.what() == what,
             TEST_WRITE_CONCATENATED("\nExpected exception ", what, "\nActual exception   ", e.what(), '\n'));
       },
       std::print(file, "hello"));
 }
 

libcxx/test/std/input.output/iostream.format/print.fun/println.file.pass.cpp

static void test_wide_stream() {
   assert(file);
 
   int mode = std::fwide(file, 1);
   assert(mode > 0);
 
   TEST_VALIDATE_EXCEPTION(
       std::system_error,
       [&]([[maybe_unused]] const std::system_error& e) {
-        [[maybe_unused]] std::string_view what{"failed to write formatted output"};
+        [[maybe_unused]] std::string_view what{"failed to write formatted output: Input/output error"};
         TEST_LIBCPP_REQUIRE(
             e.what() == what,
             TEST_WRITE_CONCATENATED("\nExpected exception ", what, "\nActual exception   ", e.what(), '\n'));
       },
       std::println(file, "hello"));
 }
 #endif // defined(TEST_HAS_GLIBC) && !defined(TEST_HAS_NO_WIDE_CHARACTERS)
 
 static void test_read_only() {
   FILE* file = fopen(filename.c_str(), "r");
   assert(file);
 
   TEST_VALIDATE_EXCEPTION(
       std::system_error,
       [&]([[maybe_unused]] const std::system_error& e) {
 #ifdef _AIX
         [[maybe_unused]] std::string_view what{"failed to write formatted output: Broken pipe"};
 #else
-        [[maybe_unused]] std::string_view what{"failed to write formatted output: Operation not permitted"};
+        [[maybe_unused]] std::string_view what{"failed to write formatted output: Bad file descriptor"};
 #endif
         TEST_LIBCPP_REQUIRE(
             e.what() == what,
             TEST_WRITE_CONCATENATED("\nExpected exception ", what, "\nActual exception   ", e.what(), '\n'));
       },
       std::println(file, "hello"));
 }
 

libcxx/test/std/input.output/iostream.format/print.fun/vprint_nonunicode.file.pass.cpp

static void test_wide_stream() {
   assert(file);
 
   int mode = std::fwide(file, 1);
   assert(mode > 0);
 
   TEST_VALIDATE_EXCEPTION(
       std::system_error,
       [&]([[maybe_unused]] const std::system_error& e) {
-        [[maybe_unused]] std::string_view what{"failed to write formatted output"};
+        [[maybe_unused]] std::string_view what{"failed to write formatted output: Input/output error"};
         TEST_LIBCPP_REQUIRE(
             e.what() == what,
             TEST_WRITE_CONCATENATED("\nExpected exception ", what, "\nActual exception   ", e.what(), '\n'));
       },
       std::vprint_nonunicode(file, "hello", std::make_format_args()));
 }
 #endif // defined(TEST_HAS_GLIBC) && !defined(TEST_HAS_NO_WIDE_CHARACTERS)
 
 static void test_read_only() {
   FILE* file = fopen(filename.c_str(), "r");
   assert(file);
 
   TEST_VALIDATE_EXCEPTION(
       std::system_error,
       [&]([[maybe_unused]] const std::system_error& e) {
 #ifdef _AIX
         [[maybe_unused]] std::string_view what{"failed to write formatted output: Broken pipe"};
 #else
-        [[maybe_unused]] std::string_view what{"failed to write formatted output: Operation not permitted"};
+        [[maybe_unused]] std::string_view what{"failed to write formatted output: Bad file descriptor"};
 #endif
         TEST_LIBCPP_REQUIRE(
             e.what() == what,
             TEST_WRITE_CONCATENATED("\nExpected exception ", what, "\nActual exception   ", e.what(), '\n'));
       },
       std::vprint_nonunicode(file, "hello", std::make_format_args()));
 }
 

libcxx/test/std/input.output/iostream.format/print.fun/vprint_unicode.file.pass.cpp

static void test_wide_stream() {
   assert(file);
 
   int mode = std::fwide(file, 1);
   assert(mode > 0);
 
   TEST_VALIDATE_EXCEPTION(
       std::system_error,
       [&]([[maybe_unused]] const std::system_error& e) {
-        [[maybe_unused]] std::string_view what{"failed to write formatted output"};
+        [[maybe_unused]] std::string_view what{"failed to write formatted output: Input/output error"};
         TEST_LIBCPP_REQUIRE(
             e.what() == what,
             TEST_WRITE_CONCATENATED("\nExpected exception ", what, "\nActual exception   ", e.what(), '\n'));
       },
       std::vprint_unicode(file, "hello", std::make_format_args()));
 }
 #endif // defined(TEST_HAS_GLIBC) && !defined(TEST_HAS_NO_WIDE_CHARACTERS)
 
 static void test_read_only() {
   FILE* file = fopen(filename.c_str(), "r");
   assert(file);
 
   TEST_VALIDATE_EXCEPTION(
       std::system_error,
       [&]([[maybe_unused]] const std::system_error& e) {
 #ifdef _AIX
         [[maybe_unused]] std::string_view what{"failed to write formatted output: Broken pipe"};
 #else
-        [[maybe_unused]] std::string_view what{"failed to write formatted output: Operation not permitted"};
+        [[maybe_unused]] std::string_view what{"failed to write formatted output: Bad file descriptor"};
 #endif
         TEST_LIBCPP_REQUIRE(
             e.what() == what,
             TEST_WRITE_CONCATENATED("\nExpected exception ", what, "\nActual exception   ", e.what(), '\n'));
       },
       std::vprint_unicode(file, "hello", std::make_format_args()));
 }