diff mbox

Three patches for std::experimental::filesystem

Message ID 20161022114714.GD2922@redhat.com
State New
Headers show

Commit Message

Jonathan Wakely Oct. 22, 2016, 11:47 a.m. UTC 
On 21/10/16 18:01 +0100, Jonathan Wakely wrote:
>    LWG2720 implement filesystem::perms::symlink_nofollow
>    
>    	* include/experimental/bits/fs_fwd.h (perms::resolve_symlinks):
>    	Replace with symlink_nofollow (LWG 2720).
>    	* src/filesystem/ops.cc (permissions(const path&, perms, error_code&)):
>    	Handle symlink_nofollow.
>    	* testsuite/experimental/filesystem/operations/create_symlink.cc: New
>    	test.
>    	* testsuite/experimental/filesystem/operations/permissions.cc: Test
>    	overload taking error_code.

GNU libc doesn't implement AT_SYMLINK_NOFOLLOW so fchmodat always
returns EOPNOTSUPP even if it's used for non-symlinks.

This patch makes us ignore symlink_nofollow if the file isn't a
symlink, so we don't get an error unless we're actually trying to
change a symlink's permissions.

Tested x86_64-linux, committed to trunk.
diff mbox

Patch

commit 5002d3b814ff6f72ce4e110079f5f406d0ff9898
Author: Jonathan Wakely <jwakely@redhat.com>
Date:   Sat Oct 22 11:57:36 2016 +0100

    Ignore perms::symlink_nofollow on non-symlinks
    
    	* src/filesystem/ops.cc (permissions(const path&, perms, error_code&)):
    	Ignore symlink_nofollow flag if file is not a symlink.
    	* testsuite/experimental/filesystem/operations/permissions.cc: Test
    	symlink_nofollow on non-symlinks.

diff --git a/libstdc++-v3/src/filesystem/ops.cc b/libstdc++-v3/src/filesystem/ops.cc
index 68343a9..2286e22 100644
--- a/libstdc++-v3/src/filesystem/ops.cc
+++ b/libstdc++-v3/src/filesystem/ops.cc
@@ -1097,7 +1097,8 @@  fs::permissions(const path& p, perms prms)
     _GLIBCXX_THROW_OR_ABORT(filesystem_error("cannot set permissions", p, ec));
 }
 
-void fs::permissions(const path& p, perms prms, error_code& ec) noexcept
+void
+fs::permissions(const path& p, perms prms, error_code& ec) noexcept
 {
   const bool add = is_set(prms, perms::add_perms);
   const bool remove = is_set(prms, perms::remove_perms);
@@ -1110,27 +1111,33 @@  void fs::permissions(const path& p, perms prms, error_code& ec) noexcept
 
   prms &= perms::mask;
 
-  if (add || remove)
+  file_status st;
+  if (add || remove || nofollow)
     {
-      auto st = nofollow ? symlink_status(p, ec) : status(p, ec);
+      st = nofollow ? symlink_status(p, ec) : status(p, ec);
       if (ec)
 	return;
       auto curr = st.permissions();
       if (add)
 	prms |= curr;
-      else
+      else if (remove)
 	prms = curr & ~prms;
     }
 
+  int err = 0;
 #if _GLIBCXX_USE_FCHMODAT
-  const int flag = nofollow ? AT_SYMLINK_NOFOLLOW : 0;
+  const int flag = (nofollow && is_symlink(st)) ? AT_SYMLINK_NOFOLLOW : 0;
   if (::fchmodat(AT_FDCWD, p.c_str(), static_cast<mode_t>(prms), flag))
+    err = errno;
 #else
-  if (nofollow)
+  if (nofollow && is_symlink(st))
     ec = std::make_error_code(std::errc::operation_not_supported);
   else if (::chmod(p.c_str(), static_cast<mode_t>(prms)))
+    err = errno;
 #endif
-    ec.assign(errno, std::generic_category());
+
+  if (err)
+    ec.assign(err, std::generic_category());
   else
     ec.clear();
 }
diff --git a/libstdc++-v3/testsuite/experimental/filesystem/operations/permissions.cc b/libstdc++-v3/testsuite/experimental/filesystem/operations/permissions.cc
index 839cfef..61471a3 100644
--- a/libstdc++-v3/testsuite/experimental/filesystem/operations/permissions.cc
+++ b/libstdc++-v3/testsuite/experimental/filesystem/operations/permissions.cc
@@ -121,6 +121,26 @@  test04()
   remove(p);
 }
 
+void
+test05()
+{
+  using perms = std::experimental::filesystem::perms;
+  std::error_code ec;
+
+  __gnu_test::scoped_file f;
+  auto p = perms::owner_write;
+
+  // symlink_nofollow should not give an error for non-symlinks
+  permissions(f.path, p|perms::symlink_nofollow, ec);
+  VERIFY( !ec );
+  auto st = status(f.path);
+  VERIFY( st.permissions() == p );
+  p |= perms::owner_read;
+  permissions(f.path, p|perms::symlink_nofollow, ec);
+  st = status(f.path);
+  VERIFY( st.permissions() == p );
+}
+
 int
 main()
 {
@@ -128,4 +148,5 @@  main()
   test02();
   test03();
   test04();
+  test05();
 }