diff mbox series

[committed] libstdc++: Avoid unconditional use of errc::not_supported [PR 99327]

Message ID YVdkuo/SLR54QKC1@redhat.com
State New
Headers show
Series [committed] libstdc++: Avoid unconditional use of errc::not_supported [PR 99327] | expand

Commit Message

Jonathan Wakely Oct. 1, 2021, 7:42 p.m. UTC 
The errc::not_supported constant is only defined if ENOTSUP is defined,
which is not true for all targets. Many uses of errc::not_supported in
the filesystem library do not actually match the intended meaning of
ENOTSUP described by POSIX. They should be using ENOSYS instead
(i.e. errc::function_not_supported).

This change ensures that appropriate error codes are used by the
filesystem library. The remaining uses of errc::not_supported are
replaced with a call to a new helper function so that an alternative
value will be used on targets that don't support errc::not_supported.

Signed-off-by: Jonathan Wakely <jwakely@redhat.com>

libstdc++-v3/ChangeLog:

	PR libstdc++/99327
	* src/filesystem/ops-common.h (__unsupported): New function to
	return a suitable error code for missing functionality.
	(posix::off_t): New typedef.
	(posix::*): Set errno to ENOSYS instead of ENOTSUP for no-op
	fallback implementations.
	(do_copy_file): Replace uses of errc::not_supported.
	* src/c++17/fs_ops.cc (fs::copy, fs::copy_file, create_dir)
	(fs::create_directory, fs::create_directory_symlink)
	(fs::create_hard_link, fs::create_symlink, fs::current_path)
	(fs::equivalent, do_stat, fs::file_size, fs::hard_link_count)
	(fs::last_write_time, fs::permissions, fs::read_symlink):
	Replace uses of errc::not_supported.
	(fs::resize_file): Qualify off_t.
	* src/filesystem/ops.cc (fs::copy, fs::copy_file, create_dir)
	(fs::create_directory, fs::create_directory_symlink)
	(fs::create_hard_link, fs::create_symlink, fs::current_path)
	(fs::equivalent, do_stat, fs::file_size, fs::last_write_time)
	(fs::permissions, fs::read_symlink, fs::system_complete):
	Replace uses of errc::not_supported.
	(fs::resize_file): Qualify off_t and enable unconditionally.
	* testsuite/19_diagnostics/system_error/cons-1.cc: Likewise.

Tested powerpc64le-linux. Committed to trunk.
commit 59ffa3e3dba5a7805585c61dd4387c5644249d52
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Tue May 11 18:47:18 2021

    libstdc++: Avoid unconditional use of errc::not_supported [PR 99327]
    
    The errc::not_supported constant is only defined if ENOTSUP is defined,
    which is not true for all targets. Many uses of errc::not_supported in
    the filesystem library do not actually match the intended meaning of
    ENOTSUP described by POSIX. They should be using ENOSYS instead
    (i.e. errc::function_not_supported).
    
    This change ensures that appropriate error codes are used by the
    filesystem library. The remaining uses of errc::not_supported are
    replaced with a call to a new helper function so that an alternative
    value will be used on targets that don't support errc::not_supported.
    
    Signed-off-by: Jonathan Wakely <jwakely@redhat.com>
    
    libstdc++-v3/ChangeLog:
    
            PR libstdc++/99327
            * src/filesystem/ops-common.h (__unsupported): New function to
            return a suitable error code for missing functionality.
            (posix::off_t): New typedef.
            (posix::*): Set errno to ENOSYS instead of ENOTSUP for no-op
            fallback implementations.
            (do_copy_file): Replace uses of errc::not_supported.
            * src/c++17/fs_ops.cc (fs::copy, fs::copy_file, create_dir)
            (fs::create_directory, fs::create_directory_symlink)
            (fs::create_hard_link, fs::create_symlink, fs::current_path)
            (fs::equivalent, do_stat, fs::file_size, fs::hard_link_count)
            (fs::last_write_time, fs::permissions, fs::read_symlink):
            Replace uses of errc::not_supported.
            (fs::resize_file): Qualify off_t.
            * src/filesystem/ops.cc (fs::copy, fs::copy_file, create_dir)
            (fs::create_directory, fs::create_directory_symlink)
            (fs::create_hard_link, fs::create_symlink, fs::current_path)
            (fs::equivalent, do_stat, fs::file_size, fs::last_write_time)
            (fs::permissions, fs::read_symlink, fs::system_complete):
            Replace uses of errc::not_supported.
            (fs::resize_file): Qualify off_t and enable unconditionally.
            * testsuite/19_diagnostics/system_error/cons-1.cc: Likewise.
diff mbox series

Patch

diff --git a/libstdc++-v3/src/c++17/fs_ops.cc b/libstdc++-v3/src/c++17/fs_ops.cc
index 4f3715bbbec..cb2dc2c617e 100644
--- a/libstdc++-v3/src/c++17/fs_ops.cc
+++ b/libstdc++-v3/src/c++17/fs_ops.cc
@@ -353,7 +353,7 @@  fs::copy(const path& from, const path& to, copy_options options,
     }
   if (is_other(f) || is_other(t))
     {
-      ec = std::make_error_code(std::errc::not_supported);
+      ec = std::make_error_code(std::errc::invalid_argument);
       return;
     }
   if (is_directory(f) && is_regular_file(t))
@@ -412,7 +412,7 @@  fs::copy(const path& from, const path& to, copy_options options,
   else
     ec.clear();
 #else
-  ec = std::make_error_code(std::errc::not_supported);
+  ec = std::make_error_code(std::errc::function_not_supported);
 #endif
 }
 
@@ -435,7 +435,7 @@  fs::copy_file(const path& from, const path& to, copy_options options,
   return do_copy_file(from.c_str(), to.c_str(), copy_file_options(options),
 		      nullptr, nullptr, ec);
 #else
-  ec = std::make_error_code(std::errc::not_supported);
+  ec = std::make_error_code(std::errc::function_not_supported);
   return false;
 #endif
 }
@@ -583,7 +583,7 @@  namespace
 	created = true;
       }
 #else
-    ec = std::make_error_code(std::errc::not_supported);
+    ec = std::make_error_code(std::errc::function_not_supported);
 #endif
     return created;
   }
@@ -631,7 +631,7 @@  fs::create_directory(const path& p, const path& attributes,
     }
   return create_dir(p, static_cast<perms>(st.st_mode), ec);
 #else
-  ec = std::make_error_code(std::errc::not_supported);
+  ec = std::make_error_code(std::errc::function_function_not_supported);
   return false;
 #endif
 }
@@ -652,7 +652,7 @@  fs::create_directory_symlink(const path& to, const path& new_symlink,
 			     error_code& ec) noexcept
 {
 #ifdef _GLIBCXX_FILESYSTEM_IS_WINDOWS
-  ec = std::make_error_code(std::errc::not_supported);
+  ec = std::make_error_code(std::errc::function_not_supported);
 #else
   create_symlink(to, new_symlink, ec);
 #endif
@@ -684,7 +684,7 @@  fs::create_hard_link(const path& to, const path& new_hard_link,
   else
     ec = __last_system_error();
 #else
-  ec = std::make_error_code(std::errc::not_supported);
+  ec = std::make_error_code(std::errc::function_not_supported);
 #endif
 }
 
@@ -708,7 +708,7 @@  fs::create_symlink(const path& to, const path& new_symlink,
   else
     ec.clear();
 #else
-  ec = std::make_error_code(std::errc::not_supported);
+  ec = std::make_error_code(std::errc::function_not_supported);
 #endif
 }
 
@@ -776,7 +776,7 @@  fs::current_path(error_code& ec)
     }
 #endif  // __GLIBC__
 #else   // _GLIBCXX_HAVE_UNISTD_H
-  ec = std::make_error_code(std::errc::not_supported);
+  ec = std::make_error_code(std::errc::function_not_supported);
 #endif
   return p;
 }
@@ -799,7 +799,7 @@  fs::current_path(const path& p, error_code& ec) noexcept
   else
     ec.clear();
 #else
-  ec = std::make_error_code(std::errc::not_supported);
+  ec = std::make_error_code(std::errc::function_not_supported);
 #endif
 }
 
@@ -839,7 +839,7 @@  fs::equivalent(const path& p1, const path& p2, error_code& ec) noexcept
     {
       if (is_other(s1) && is_other(s2))
 	{
-	  ec = std::make_error_code(std::errc::not_supported);
+	  ec = std::__unsupported();
 	  return false;
 	}
       ec.clear();
@@ -897,7 +897,7 @@  fs::equivalent(const path& p1, const path& p2, error_code& ec) noexcept
     ec.clear();
   return false;
 #else
-  ec = std::make_error_code(std::errc::not_supported);
+  ec = std::make_error_code(std::errc::function_not_supported);
 #endif
   return false;
 }
@@ -928,7 +928,7 @@  namespace
       ec.clear();
       return f(st);
 #else
-      ec = std::make_error_code(std::errc::not_supported);
+      ec = std::make_error_code(std::errc::function_not_supported);
       return deflt;
 #endif
     }
@@ -953,10 +953,10 @@  fs::file_size(const path& p, error_code& ec) noexcept
       if (s.type == file_type::directory)
 	ec = std::make_error_code(std::errc::is_a_directory);
       else
-	ec = std::make_error_code(std::errc::not_supported);
+	ec = std::__unsupported();
     }
 #else
-  ec = std::make_error_code(std::errc::not_supported);
+  ec = std::make_error_code(std::errc::function_not_supported);
 #endif
   return -1;
 }
@@ -978,7 +978,7 @@  fs::hard_link_count(const path& p, error_code& ec) noexcept
   return do_stat(p, ec, std::mem_fn(&stat_type::st_nlink),
 		 static_cast<uintmax_t>(-1));
 #else
-  ec = std::make_error_code(std::errc::not_supported);
+  ec = std::make_error_code(std::errc::function_not_supported);
   return static_cast<uintmax_t>(-1);
 #endif
 }
@@ -1026,7 +1026,7 @@  fs::last_write_time(const path& p, error_code& ec) noexcept
 		 },
 		 file_time_type::min());
 #else
-  ec = std::make_error_code(std::errc::not_supported);
+  ec = std::make_error_code(std::errc::function_not_supported);
   return file_time_type::min();
 #endif
 }
@@ -1072,7 +1072,7 @@  fs::last_write_time(const path& p,
   else
     ec.clear();
 #else
-  ec = std::make_error_code(std::errc::not_supported);
+  ec = std::make_error_code(std::errc::function_not_supported);
 #endif
 }
 
@@ -1121,7 +1121,7 @@  fs::permissions(const path& p, perms prms, perm_options opts,
     err = errno;
 #else
   if (nofollow && is_symlink(st))
-    ec = std::make_error_code(std::errc::not_supported);
+    ec = std::__unsupported();
   else if (posix::chmod(p.c_str(), static_cast<posix::mode_t>(prms)))
     err = errno;
 #endif
@@ -1206,7 +1206,7 @@  fs::path fs::read_symlink(const path& p, error_code& ec)
     }
   while (true);
 #else
-  ec = std::make_error_code(std::errc::not_supported);
+  ec = std::make_error_code(std::errc::function_not_supported);
 #endif
   return result;
 }
@@ -1434,7 +1434,7 @@  fs::resize_file(const path& p, uintmax_t size)
 void
 fs::resize_file(const path& p, uintmax_t size, error_code& ec) noexcept
 {
-  if (size > static_cast<uintmax_t>(std::numeric_limits<off_t>::max()))
+  if (size > static_cast<uintmax_t>(std::numeric_limits<posix::off_t>::max()))
     ec.assign(EINVAL, std::generic_category());
   else if (posix::truncate(p.c_str(), size))
     ec.assign(errno, std::generic_category());
diff --git a/libstdc++-v3/src/filesystem/ops-common.h b/libstdc++-v3/src/filesystem/ops-common.h
index e999e11b422..90ebeba7f01 100644
--- a/libstdc++-v3/src/filesystem/ops-common.h
+++ b/libstdc++-v3/src/filesystem/ops-common.h
@@ -69,6 +69,30 @@  _GLIBCXX_BEGIN_NAMESPACE_VERSION
 #endif
   }
 
+  // Get an error code indicating unsupported functionality.
+  //
+  // This should be used when a function is unable to behave as specified
+  // due to an incomplete or partial implementation, e.g.
+  // filesystem::equivalent(a, b) if is_other(a) && is_other(b) is true.
+  //
+  // Use errc::function_not_supported for functions that are entirely
+  // unimplemented, e.g. create_symlink on Windows.
+  //
+  // Use errc::invalid_argument for requests to perform operations outside
+  // the spec, e.g. trying to copy a directory using filesystem::copy_file.
+  inline error_code
+  __unsupported() noexcept
+  {
+#if defined ENOTSUP
+    return std::make_error_code(std::errc::not_supported);
+#elif defined EOPNOTSUPP
+    // This is supposed to be for socket operations
+    return std::make_error_code(std::errc::operation_not_supported);
+#else
+    return std::make_error_code(std::errc::invalid_argument);
+#endif
+  }
+
 namespace filesystem
 {
 namespace __gnu_posix
@@ -128,6 +152,7 @@  namespace __gnu_posix
     return -1;
   }
 
+  using off_t = _off64_t;
   inline int truncate(const wchar_t* path, _off64_t length)
   {
     const int fd = ::_wopen(path, _O_BINARY|_O_RDWR);
@@ -164,6 +189,7 @@  namespace __gnu_posix
   using ::utime;
 # endif
   using ::rename;
+  using ::off_t;
 # ifdef _GLIBCXX_HAVE_TRUNCATE
   using ::truncate;
 # else
@@ -183,15 +209,16 @@  namespace __gnu_posix
 # endif
   using char_type = char;
 #else // ! _GLIBCXX_FILESYSTEM_IS_WINDOWS && ! _GLIBCXX_HAVE_UNISTD_H
-  inline int open(const char*, int, ...) { errno = ENOTSUP; return -1; }
-  inline int close(int) { errno = ENOTSUP; return -1; }
+  inline int open(const char*, int, ...) { errno = ENOSYS; return -1; }
+  inline int close(int) { errno = ENOSYS; return -1; }
   using mode_t = int;
-  inline int chmod(const char*, mode_t) { errno = ENOTSUP; return -1; }
-  inline int mkdir(const char*, mode_t) { errno = ENOTSUP; return -1; }
-  inline char* getcwd(char*, size_t) { errno = ENOTSUP; return nullptr; }
-  inline int chdir(const char*) { errno = ENOTSUP; return -1; }
-  inline int rename(const char*, const char*) { errno = ENOTSUP; return -1; }
-  inline int truncate(const char*, long) { errno = ENOTSUP; return -1; }
+  inline int chmod(const char*, mode_t) { errno = ENOSYS; return -1; }
+  inline int mkdir(const char*, mode_t) { errno = ENOSYS; return -1; }
+  inline char* getcwd(char*, size_t) { errno = ENOSYS; return nullptr; }
+  inline int chdir(const char*) { errno = ENOSYS; return -1; }
+  inline int rename(const char*, const char*) { errno = ENOSYS; return -1; }
+  using off_t = long;
+  inline int truncate(const char*, off_t) { errno = ENOSYS; return -1; }
   using char_type = char;
 #endif // _GLIBCXX_FILESYSTEM_IS_WINDOWS
 } // namespace __gnu_posix
@@ -374,7 +401,7 @@  _GLIBCXX_BEGIN_NAMESPACE_FILESYSTEM
     // 2712. copy_file() has a number of unspecified error conditions
     if (!is_regular_file(f))
       {
-	ec = std::make_error_code(std::errc::not_supported);
+	ec = std::make_error_code(std::errc::invalid_argument);
 	return false;
       }
 
@@ -382,7 +409,7 @@  _GLIBCXX_BEGIN_NAMESPACE_FILESYSTEM
       {
 	if (!is_regular_file(t))
 	  {
-	    ec = std::make_error_code(std::errc::not_supported);
+	    ec = std::make_error_code(std::errc::invalid_argument);
 	    return false;
 	  }
 
@@ -413,7 +440,7 @@  _GLIBCXX_BEGIN_NAMESPACE_FILESYSTEM
 	  }
 	else if (!is_regular_file(t))
 	  {
-	    ec = std::make_error_code(std::errc::not_supported);
+	    ec = std::make_error_code(std::errc::invalid_argument);
 	    return false;
 	  }
       }
@@ -572,7 +599,7 @@  _GLIBCXX_BEGIN_NAMESPACE_FILESYSTEM
     else
       ec = std::last_system_error();
 #else
-    ec = std::make_error_code(std::errc::not_supported);
+    ec = std::make_error_code(std::errc::function_not_supported);
 #endif
   }
 #pragma GCC diagnostic pop
diff --git a/libstdc++-v3/src/filesystem/ops.cc b/libstdc++-v3/src/filesystem/ops.cc
index cc7117b0cd1..94b4123b878 100644
--- a/libstdc++-v3/src/filesystem/ops.cc
+++ b/libstdc++-v3/src/filesystem/ops.cc
@@ -293,7 +293,7 @@  fs::copy(const path& from, const path& to, copy_options options,
     }
   if (is_other(f) || is_other(t))
     {
-      ec = std::make_error_code(std::errc::not_supported);
+      ec = std::make_error_code(std::errc::invalid_argument);
       return;
     }
   if (is_directory(f) && is_regular_file(t))
@@ -372,7 +372,7 @@  fs::copy_file(const path& from, const path& to, copy_options options,
   return do_copy_file(from.c_str(), to.c_str(), copy_file_options(options),
 		      nullptr, nullptr, ec);
 #else
-  ec = std::make_error_code(std::errc::not_supported);
+  ec = std::make_error_code(std::errc::function_not_supported);
   return false;
 #endif
 }
@@ -491,7 +491,7 @@  namespace
 	created = true;
       }
 #else
-    ec = std::make_error_code(std::errc::not_supported);
+    ec = std::make_error_code(std::errc::function_not_supported);
 #endif
     return created;
   }
@@ -539,7 +539,7 @@  fs::create_directory(const path& p, const path& attributes,
     }
   return create_dir(p, static_cast<perms>(st.st_mode), ec);
 #else
-  ec = std::make_error_code(std::errc::not_supported);
+  ec = std::make_error_code(std::errc::function_not_supported);
   return false;
 #endif
 }
@@ -560,7 +560,7 @@  fs::create_directory_symlink(const path& to, const path& new_symlink,
 			     error_code& ec) noexcept
 {
 #ifdef _GLIBCXX_FILESYSTEM_IS_WINDOWS
-  ec = std::make_error_code(std::errc::not_supported);
+  ec = std::make_error_code(std::errc::function_not_supported);
 #else
   create_symlink(to, new_symlink, ec);
 #endif
@@ -592,7 +592,7 @@  fs::create_hard_link(const path& to, const path& new_hard_link,
   else
     ec = __last_system_error();
 #else
-  ec = std::make_error_code(std::errc::not_supported);
+  ec = std::make_error_code(std::errc::function_not_supported);
 #endif
 }
 
@@ -616,7 +616,7 @@  fs::create_symlink(const path& to, const path& new_symlink,
   else
     ec.clear();
 #else
-  ec = std::make_error_code(std::errc::not_supported);
+  ec = std::make_error_code(std::errc::function_not_supported);
 #endif
 }
 
@@ -683,7 +683,7 @@  fs::current_path(error_code& ec)
     }
 #endif  // __GLIBC__
 #else   // _GLIBCXX_HAVE_UNISTD_H
-  ec = std::make_error_code(std::errc::not_supported);
+  ec = std::make_error_code(std::errc::function_not_supported);
 #endif
   return p;
 }
@@ -706,7 +706,7 @@  fs::current_path(const path& p, error_code& ec) noexcept
   else
     ec.clear();
 #else
-  ec = std::make_error_code(std::errc::not_supported);
+  ec = std::make_error_code(std::errc::function_not_supported);
 #endif
 }
 
@@ -746,7 +746,7 @@  fs::equivalent(const path& p1, const path& p2, error_code& ec) noexcept
     {
       if (is_other(s1) && is_other(s2))
 	{
-	  ec = std::make_error_code(std::errc::not_supported);
+	  ec = std::__unsupported();
 	  return false;
 	}
       ec.clear();
@@ -762,7 +762,7 @@  fs::equivalent(const path& p1, const path& p2, error_code& ec) noexcept
     ec.clear();
   return false;
 #else
-  ec = std::make_error_code(std::errc::not_supported);
+  ec = std::make_error_code(std::errc::function_not_supported);
 #endif
   return false;
 }
@@ -793,7 +793,7 @@  namespace
       ec.clear();
       return f(st);
 #else
-      ec = std::make_error_code(std::errc::not_supported);
+      ec = std::make_error_code(std::errc::function_not_supported);
       return deflt;
 #endif
     }
@@ -817,7 +817,7 @@  fs::file_size(const path& p, error_code& ec) noexcept
       if (s.type == file_type::directory)
 	ec = std::make_error_code(std::errc::is_a_directory);
       else
-	ec = std::make_error_code(std::errc::not_supported);
+	ec = std::__unsupported();
     }
   return -1;
 }
@@ -920,7 +920,7 @@  fs::last_write_time(const path& p __attribute__((__unused__)),
   else
     ec.clear();
 #else
-  ec = std::make_error_code(std::errc::not_supported);
+  ec = std::make_error_code(std::errc::function_not_supported);
 #endif
 }
 
@@ -967,7 +967,7 @@  fs::permissions(const path& p, perms prms, error_code& ec) noexcept
     err = errno;
 #else
   if (nofollow && is_symlink(st))
-    ec = std::make_error_code(std::errc::operation_not_supported);
+    ec = std::__unsupported();
   else if (posix::chmod(p.c_str(), static_cast<mode_t>(prms)))
     err = errno;
 #endif
@@ -1032,7 +1032,7 @@  fs::path fs::read_symlink(const path& p [[gnu::unused]], error_code& ec)
     }
   while (true);
 #else
-  ec = std::make_error_code(std::errc::not_supported);
+  ec = std::make_error_code(std::errc::function_not_supported);
 #endif
   return result;
 }
@@ -1153,16 +1153,12 @@  fs::resize_file(const path& p, uintmax_t size)
 void
 fs::resize_file(const path& p, uintmax_t size, error_code& ec) noexcept
 {
-#ifdef _GLIBCXX_HAVE_UNISTD_H
-  if (size > static_cast<uintmax_t>(std::numeric_limits<off_t>::max()))
+  if (size > static_cast<uintmax_t>(std::numeric_limits<posix::off_t>::max()))
     ec.assign(EINVAL, std::generic_category());
   else if (posix::truncate(p.c_str(), size))
     ec.assign(errno, std::generic_category());
   else
     ec.clear();
-#else
-  ec = std::make_error_code(std::errc::not_supported);
-#endif
 }
 
 
@@ -1280,7 +1276,7 @@  fs::system_complete(const path& p, error_code& ec)
       || p.root_name() == base.root_name())
     return absolute(p, base);
   // else TODO
-  ec = std::make_error_code(std::errc::not_supported);
+  ec = std::__unsupported();
   return {};
 #else
   if (ec.value())
diff --git a/libstdc++-v3/testsuite/19_diagnostics/system_error/cons-1.cc b/libstdc++-v3/testsuite/19_diagnostics/system_error/cons-1.cc
index effb6709f8b..168a6fca2a9 100644
--- a/libstdc++-v3/testsuite/19_diagnostics/system_error/cons-1.cc
+++ b/libstdc++-v3/testsuite/19_diagnostics/system_error/cons-1.cc
@@ -26,19 +26,19 @@  int main()
 {
   const std::string s("too late: boulangerie out of pain au raisin");
   const std::error_code
-    e(std::make_error_code(std::errc::operation_not_supported));
+    e(std::make_error_code(std::errc::invalid_argument));
 
   // 1
   {
     std::system_error err1(e, s);
-    VERIFY( err1.code() == e ); 
+    VERIFY( err1.code() == e );
     VERIFY( std::string(err1.what()).find(s) != std::string::npos );
   }
 
   // 2
   {
     std::system_error err2(95, std::system_category(), s);
-    VERIFY( err2.code() == std::error_code(95, std::system_category()) ); 
+    VERIFY( err2.code() == std::error_code(95, std::system_category()) );
     VERIFY( std::string((err2.what(), s)).find(s) != std::string::npos );
   }