diff --git a/libcxx/include/__config b/libcxx/include/__config --- a/libcxx/include/__config +++ b/libcxx/include/__config @@ -1458,6 +1458,13 @@ # define _LIBCPP_INIT_PRIORITY_MAX #endif +#if defined(__GNUC__) || defined(__clang__) +# define _LIBCPP_FORMAT_PRINTF(...) \ + __attribute__((__format__(__printf__, __VA_ARGS__))) +#else +# define _LIBCPP_FORMAT_PRINTF(...) +#endif + #endif // __cplusplus #endif // _LIBCPP_CONFIG diff --git a/libcxx/src/filesystem/directory_iterator.cpp b/libcxx/src/filesystem/directory_iterator.cpp --- a/libcxx/src/filesystem/directory_iterator.cpp +++ b/libcxx/src/filesystem/directory_iterator.cpp @@ -272,7 +272,7 @@ path root = move(__imp_->__root_); __imp_.reset(); if (m_ec) - err.report(m_ec, "at root \"%s\"", root); + err.report(m_ec, "at root " PATH_CSTR_FMT, root.c_str()); } return *this; } @@ -359,7 +359,7 @@ 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 " PATH_CSTR_FMT, root.c_str()); } else { __imp_.reset(); } @@ -404,7 +404,7 @@ } 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 " PATH_CSTR_FMT, at_ent.c_str()); } } return false; diff --git a/libcxx/src/filesystem/filesystem_common.h b/libcxx/src/filesystem/filesystem_common.h --- a/libcxx/src/filesystem/filesystem_common.h +++ b/libcxx/src/filesystem/filesystem_common.h @@ -42,8 +42,10 @@ #if defined(_LIBCPP_WIN32API) #define PS(x) (L##x) +#define PATH_CSTR_FMT "\"%ls\"" #else #define PS(x) (x) +#define PATH_CSTR_FMT "\"%s\"" #endif _LIBCPP_BEGIN_NAMESPACE_FILESYSTEM @@ -57,68 +59,43 @@ namespace { -static string format_string_imp(const char* msg, ...) { - // we might need a second shot at this, so pre-emptivly make a copy - struct GuardVAList { - va_list& target; - bool active = true; - GuardVAList(va_list& tgt) : target(tgt), active(true) {} - void clear() { - if (active) - va_end(target); - active = false; - } - ~GuardVAList() { - if (active) - va_end(target); - } - }; - va_list args; - va_start(args, msg); - GuardVAList args_guard(args); +static _LIBCPP_FORMAT_PRINTF(1, 0) string +format_string_impl(const char* msg, va_list ap) { + array buf; - va_list args_cp; - va_copy(args_cp, args); - GuardVAList args_copy_guard(args_cp); + va_list apcopy; + va_copy(apcopy, ap); + int ret = ::vsnprintf(buf.data(), buf.size(), msg, apcopy); + va_end(apcopy); std::string result; - - array local_buff; - size_t size_with_null = local_buff.size(); - auto ret = ::vsnprintf(local_buff.data(), size_with_null, msg, args_cp); - - args_copy_guard.clear(); - - // handle empty expansion - if (ret == 0) - return result; - if (static_cast(ret) < size_with_null) { - result.assign(local_buff.data(), static_cast(ret)); - return result; + if (static_cast(ret) < buf.size()) { + result.assign(buf.data(), static_cast(ret)); + } else { + // we did not provide a long enough buffer on our first attempt. The + // return value is the number of bytes (excluding the null byte) that are + // needed for formatting. + size_t size_with_null = static_cast(ret) + 1; + result.__resize_default_init(size_with_null - 1); + ret = ::vsnprintf(&result[0], size_with_null, msg, ap); + _LIBCPP_ASSERT(static_cast(ret) == (size_with_null - 1), "TODO"); } - - // we did not provide a long enough buffer on our first attempt. The - // return value is the number of bytes (excluding the null byte) that are - // needed for formatting. - size_with_null = static_cast(ret) + 1; - result.__resize_default_init(size_with_null - 1); - ret = ::vsnprintf(&result[0], size_with_null, msg, args); - _LIBCPP_ASSERT(static_cast(ret) == (size_with_null - 1), "TODO"); - return result; } -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 -Arg const& unwrap(Arg const& a) { - static_assert(!is_class::value, "cannot pass class here"); - return a; -} - -template -string format_string(const char* fmt, Args const&... args) { - return format_string_imp(fmt, unwrap(args)...); +static _LIBCPP_FORMAT_PRINTF(1, 2) +string format_string(const char* msg, ...) { + std::string ret; + va_list ap; + va_start(ap, msg); + try { + ret = format_string_impl(msg, ap); + } catch (...) { + va_end(ap); + throw; + } + va_end(ap); + return ret; } error_code capture_errno() { @@ -190,14 +167,14 @@ _LIBCPP_UNREACHABLE(); } - template - T report(const error_code& ec, const char* msg, Args const&... args) const { + _LIBCPP_FORMAT_PRINTF(3, 0) + void report_impl(const error_code& ec, const char* msg, va_list ap) const { if (ec_) { *ec_ = ec; - return error_value(); + return; } string what = - string("in ") + func_name_ + ": " + format_string(msg, args...); + string("in ") + func_name_ + ": " + format_string_impl(msg, ap); switch (bool(p1_) + bool(p2_)) { case 0: __throw_filesystem_error(what, ec); @@ -209,11 +186,36 @@ _LIBCPP_UNREACHABLE(); } - T report(errc const& err) const { return report(make_error_code(err)); } + _LIBCPP_FORMAT_PRINTF(3, 4) + T report(const error_code& ec, const char* msg, ...) const { + va_list ap; + va_start(ap, msg); + try { + report_impl(ec, msg, ap); + } catch (...) { + va_end(ap); + throw; + } + va_end(ap); + return error_value(); + } + + T report(errc const& err) const { + return report(make_error_code(err)); + } - template - T report(errc const& err, const char* msg, Args const&... args) const { - return report(make_error_code(err), msg, args...); + _LIBCPP_FORMAT_PRINTF(3, 4) + T report(errc const& err, const char* msg, ...) const { + va_list ap; + va_start(ap, msg); + try { + report_impl(make_error_code(err), msg, ap); + } catch (...) { + va_end(ap); + throw; + } + va_end(ap); + return error_value(); } private: diff --git a/libcxx/src/filesystem/operations.cpp b/libcxx/src/filesystem/operations.cpp --- a/libcxx/src/filesystem/operations.cpp +++ b/libcxx/src/filesystem/operations.cpp @@ -667,27 +667,20 @@ 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: + case 0: return detail::format_string("filesystem error: %s", derived_what); case 1: - return detail::format_string("filesystem error: %s [" PS_FMT "]", derived_what, - p1); + return detail::format_string("filesystem error: %s [" PATH_CSTR_FMT "]", derived_what, + path1().c_str()); case 2: - return detail::format_string("filesystem error: %s [" PS_FMT "] [" PS_FMT "]", - derived_what, p1, p2); + return detail::format_string("filesystem error: %s [" PATH_CSTR_FMT "] [" PATH_CSTR_FMT "]", + derived_what, path1().c_str(), path2().c_str()); } + _LIBCPP_UNREACHABLE(); }(); } @@ -1455,11 +1448,11 @@ error_code m_ec; file_status st = detail::posix_stat(p, &m_ec); if (!status_known(st)) - return err.report(m_ec, "cannot access path \"" PS_FMT "\"", p); + return err.report(m_ec, "cannot access path " PATH_CSTR_FMT, p.c_str()); if (!exists(st) || !is_directory(st)) - return err.report(errc::not_a_directory, "path \"" PS_FMT "\" is not a directory", - p); + return err.report(errc::not_a_directory, + "path " PATH_CSTR_FMT " is not a directory", p.c_str()); return p; } diff --git a/libcxx/test/support/filesystem_test_helper.h b/libcxx/test/support/filesystem_test_helper.h --- a/libcxx/test/support/filesystem_test_helper.h +++ b/libcxx/test/support/filesystem_test_helper.h @@ -628,9 +628,7 @@ additional_msg = opt_message + ": "; } auto transform_path = [](const fs::path& p) { - if (p.native().empty()) - return std::string("\"\""); - return p.string(); + return "\"" + p.string() + "\""; }; std::string format = [&]() -> std::string { switch (num_paths) {