[libcxx] Fix the format strings for formatting paths in directory_iterator.cpp

[• Quuxplusone]

libcxx/src/filesystem/filesystem_common.h

115

How about moving the escaped-double-quotes into this format string?

#define PS_FMT "\"%ls\""
#define PS_FMT "\"%s\""

That'll make the call-sites look less wacky.

(I admit it doesn't really make any sense that we double-quote these paths in the first place. It's not as if we're going to actually escape the backslashes/quotes in the paths; so the quotation marks are kind of a lie. But I don't feel like unilaterally suggesting their complete removal.)

[curdeius]

libcxx/src/filesystem/filesystem_common.h

115

Personally I'd go the opposite way and rather see these macros closer to PRIxYYx macros (cf. https://en.cppreference.com/w/c/types/integer) and so not even include the % sign. This way, they could be used in the future with additional format options, like width, between % and the macro.

#define PS_FMT "ls"
#define PS_FMT "s"

And put % in the format string. WDYT?

[mstorsjo]

libcxx/src/filesystem/filesystem_common.h

115

Something along those lines were my original intent, but I don't object to going the other way and including more parts in the macro.

I'll leave resolving the rest of the bits in D98097 to you, thanks!

[mstorsjo]

Superseded by D98097.

libcxx/src/filesystem/directory_iterator.cpp

directory_iterator& directory_iterator::__increment(error_code* ec) {
   _LIBCPP_ASSERT(__imp_, "Attempting to increment an invalid iterator");
   ErrorHandler<void> err("directory_iterator::operator++()", ec);
 
   error_code m_ec;
   if (!__imp_->advance(m_ec)) {
     path root = move(__imp_->__root_);
     __imp_.reset();
     if (m_ec)
-      err.report(m_ec, "at root \"%s\"", root);
+      err.report(m_ec, "at root \"" PS_FMT "\"", root);
   }
   return *this;
 }
 
 directory_entry const& directory_iterator::__dereference() const {
   _LIBCPP_ASSERT(__imp_, "Attempting to dereference an invalid iterator");
   return __imp_->__entry_;
 }
while (stack.size() > 0) {
     if (m_ec)
       break;
     stack.pop();
   }
 
   if (m_ec) {
     path root = move(stack.top().__root_);
     __imp_.reset();
-    err.report(m_ec, "at root \"%s\"", root);
+    err.report(m_ec, "at root \"" PS_FMT "\"", root);
   } else {
     __imp_.reset();
   }
 }
 
 bool recursive_directory_iterator::__try_recursion(error_code* ec) {
   ErrorHandler<void> err("recursive_directory_iterator::operator++()", ec);
 
if (m_ec) {
     const bool allow_eacess =
         bool(__imp_->__options_ & directory_options::skip_permission_denied);
     if (m_ec.value() == EACCES && allow_eacess) {
       if (ec)
         ec->clear();
     } else {
       path at_ent = move(curr_it.__entry_.__p_);
       __imp_.reset();
-      err.report(m_ec, "attempting recursion into \"%s\"", at_ent);
+      err.report(m_ec, "attempting recursion into \"" PS_FMT "\"", at_ent);
     }
   }
   return false;
 }
 
 _LIBCPP_END_NAMESPACE_FILESYSTEM

libcxx/src/filesystem/filesystem_common.h

static string format_string_imp(const char* msg, ...) {
   size_with_null = static_cast<size_t>(ret) + 1;
   result.__resize_default_init(size_with_null - 1);
   ret = ::vsnprintf(&result[0], size_with_null, msg, args);
   _LIBCPP_ASSERT(static_cast<size_t>(ret) == (size_with_null - 1), "TODO");
 
   return result;
 }
 
+#if defined(_LIBCPP_WIN32API)
+#define PS_FMT "%ls"
+#else
+#define PS_FMT "%s"
+#endif
+
 const path::value_type* unwrap(path::string_type const& s) { return s.c_str(); }
 const path::value_type* unwrap(path const& p) { return p.native().c_str(); }
 template <class Arg>
 Arg const& unwrap(Arg const& a) {
   static_assert(!is_class<Arg>::value, "cannot pass class here");
   return a;
 }
 

libcxx/src/filesystem/operations.cpp

#else
   timeval tv;
   gettimeofday(&tv, 0);
   return time_point(__secs(tv.tv_sec) + __microsecs(tv.tv_usec));
 #endif // CLOCK_REALTIME
 }
 
 filesystem_error::~filesystem_error() {}
 
-#if defined(_LIBCPP_WIN32API)
-#define PS_FMT "%ls"
-#else
-#define PS_FMT "%s"
-#endif
-
 void filesystem_error::__create_what(int __num_paths) {
   const char* derived_what = system_error::what();
   __storage_->__what_ = [&]() -> string {
     const path::value_type* p1 = path1().native().empty() ? PS("\"\"") : path1().c_str();
     const path::value_type* p2 = path2().native().empty() ? PS("\"\"") : path2().c_str();
     switch (__num_paths) {
     default:
       return detail::format_string("filesystem error: %s", derived_what);