[libcxx] r328476 - Implement filesystem::perm_options specified in NB comments.

Eric Fiselier via cfe-commits cfe-commits at lists.llvm.org
Sun Mar 25 23:23:56 PDT 2018


Author: ericwf
Date: Sun Mar 25 23:23:55 2018
New Revision: 328476

URL: http://llvm.org/viewvc/llvm-project?rev=328476&view=rev
Log:
Implement filesystem::perm_options specified in NB comments.

The NB comments for filesystem changed permissions and added
a new enum `perm_options` which control how the permissions
are applied.

This implements than NB resolution

Modified:
    libcxx/trunk/include/experimental/filesystem
    libcxx/trunk/src/experimental/filesystem/operations.cpp
    libcxx/trunk/test/std/experimental/filesystem/class.rec.dir.itr/rec.dir.itr.members/increment.pass.cpp
    libcxx/trunk/test/std/experimental/filesystem/fs.enum/enum.perms.pass.cpp
    libcxx/trunk/test/std/experimental/filesystem/fs.op.funcs/fs.op.create_directory/create_directory_with_attributes.pass.cpp
    libcxx/trunk/test/std/experimental/filesystem/fs.op.funcs/fs.op.permissions/permissions.pass.cpp

Modified: libcxx/trunk/include/experimental/filesystem
URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/include/experimental/filesystem?rev=328476&r1=328475&r2=328476&view=diff
==============================================================================
--- libcxx/trunk/include/experimental/filesystem (original)
+++ libcxx/trunk/include/experimental/filesystem Sun Mar 25 23:23:55 2018
@@ -68,6 +68,7 @@
 
     enum class file_type;
     enum class perms;
+    enum class perm_options;
     enum class copy_options;
     enum class directory_options;
 
@@ -178,8 +179,11 @@
     void last_write_time(const path& p, file_time_type new_time,
                                  error_code& ec) _NOEXCEPT;
 
-    void permissions(const path& p, perms prms);
-    void permissions(const path& p, perms prms, error_code& ec) _NOEXCEPT;
+    void permissions(const path& p, perms prms,
+                     perm_options opts=perm_options::replace);
+    void permissions(const path& p, perms prms, error_code& ec) noexcept;
+    void permissions(const path& p, perms prms, perm_options opts,
+                     error_code& ec);
 
     path read_symlink(const path& p);
     path read_symlink(const path& p, error_code& ec);
@@ -290,10 +294,6 @@ enum class _LIBCPP_ENUM_VIS perms : unsi
     sticky_bit   = 01000,
     mask         = 07777,
     unknown      = 0xFFFF,
-
-    add_perms        = 0x10000,
-    remove_perms     = 0x20000,
-    symlink_nofollow = 0x40000
 };
 
 _LIBCPP_INLINE_VISIBILITY
@@ -324,6 +324,41 @@ _LIBCPP_INLINE_VISIBILITY
 inline perms& operator^=(perms& _LHS, perms _RHS)
 { return _LHS = _LHS ^ _RHS; }
 
+enum class _LIBCPP_ENUM_VIS perm_options : unsigned char {
+  replace  = 1,
+  add      = 2,
+  remove   = 4,
+  nofollow = 8
+};
+
+_LIBCPP_INLINE_VISIBILITY
+inline _LIBCPP_CONSTEXPR perm_options operator&(perm_options _LHS, perm_options _RHS)
+{ return static_cast<perm_options>(static_cast<unsigned>(_LHS) & static_cast<unsigned>(_RHS)); }
+
+_LIBCPP_INLINE_VISIBILITY
+inline _LIBCPP_CONSTEXPR perm_options operator|(perm_options _LHS, perm_options _RHS)
+{ return static_cast<perm_options>(static_cast<unsigned>(_LHS) | static_cast<unsigned>(_RHS)); }
+
+_LIBCPP_INLINE_VISIBILITY
+inline _LIBCPP_CONSTEXPR perm_options operator^(perm_options _LHS, perm_options _RHS)
+{ return static_cast<perm_options>(static_cast<unsigned>(_LHS) ^ static_cast<unsigned>(_RHS)); }
+
+_LIBCPP_INLINE_VISIBILITY
+inline _LIBCPP_CONSTEXPR perm_options operator~(perm_options _LHS)
+{ return static_cast<perm_options>(~static_cast<unsigned>(_LHS)); }
+
+_LIBCPP_INLINE_VISIBILITY
+inline perm_options& operator&=(perm_options& _LHS, perm_options _RHS)
+{ return _LHS = _LHS & _RHS; }
+
+_LIBCPP_INLINE_VISIBILITY
+inline perm_options& operator|=(perm_options& _LHS, perm_options _RHS)
+{ return _LHS = _LHS | _RHS; }
+
+_LIBCPP_INLINE_VISIBILITY
+inline perm_options& operator^=(perm_options& _LHS, perm_options _RHS)
+{ return _LHS = _LHS ^ _RHS; }
+
 enum class _LIBCPP_ENUM_VIS copy_options : unsigned short
 {
     none               = 0,
@@ -1286,7 +1321,7 @@ _LIBCPP_FUNC_VIS
 void __last_write_time(const path& p, file_time_type new_time,
         error_code *ec=nullptr);
 _LIBCPP_FUNC_VIS
-void __permissions(const path& p, perms prms, error_code *ec=nullptr);
+void __permissions(const path&, perms, perm_options, error_code* = nullptr);
 _LIBCPP_FUNC_VIS
 path __read_symlink(const path& p, error_code *ec=nullptr);
 _LIBCPP_FUNC_VIS
@@ -1664,13 +1699,20 @@ void last_write_time(const path& __p, fi
 }
 
 inline _LIBCPP_INLINE_VISIBILITY
-void permissions(const path& __p, perms __prms) {
-    __permissions(__p, __prms);
+void permissions(const path& __p, perms __prms,
+                 perm_options __opts = perm_options::replace) {
+    __permissions(__p, __prms, __opts);
+}
+
+inline _LIBCPP_INLINE_VISIBILITY
+void permissions(const path& __p, perms __prms, error_code& __ec) _NOEXCEPT {
+    __permissions(__p, __prms, perm_options::replace, &__ec);
 }
 
 inline _LIBCPP_INLINE_VISIBILITY
-void permissions(const path& __p, perms __prms, error_code& __ec)  {
-    __permissions(__p, __prms, &__ec);
+void permissions(const path& __p, perms __prms, perm_options __opts,
+                 error_code& __ec)  {
+    __permissions(__p, __prms, __opts, &__ec);
 }
 
 inline _LIBCPP_INLINE_VISIBILITY

Modified: libcxx/trunk/src/experimental/filesystem/operations.cpp
URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/src/experimental/filesystem/operations.cpp?rev=328476&r1=328475&r2=328476&view=diff
==============================================================================
--- libcxx/trunk/src/experimental/filesystem/operations.cpp (original)
+++ libcxx/trunk/src/experimental/filesystem/operations.cpp Sun Mar 25 23:23:55 2018
@@ -178,7 +178,7 @@ bool copy_file_impl(const path& from, co
                      ec, "copy_file", from, to);
         return false;
     }
-    __permissions(to, from_perms, ec);
+    __permissions(to, from_perms, perm_options::replace, ec);
     // TODO what if permissions fails?
     return true;
 }
@@ -635,14 +635,17 @@ void __last_write_time(const path& p, fi
 }
 
 
-void __permissions(const path& p, perms prms, std::error_code *ec)
+void __permissions(const path& p, perms prms, perm_options opts,
+                   std::error_code *ec)
 {
-
-    const bool resolve_symlinks = !bool(perms::symlink_nofollow & prms);
-    const bool add_perms = bool(perms::add_perms & prms);
-    const bool remove_perms = bool(perms::remove_perms & prms);
-    _LIBCPP_ASSERT(!(add_perms && remove_perms),
-                   "Both add_perms and remove_perms are set");
+    auto has_opt = [&](perm_options o) { return bool(o & opts); };
+    const bool resolve_symlinks = !has_opt(perm_options::nofollow);
+    const bool add_perms = has_opt(perm_options::add);
+    const bool remove_perms = has_opt(perm_options::remove);
+    _LIBCPP_ASSERT(
+       (add_perms + remove_perms + has_opt(perm_options::replace)) == 1,
+       "One and only one of the perm_options constants replace, add, or remove "
+        "is present in opts");
 
     bool set_sym_perms = false;
     prms &= perms::mask;

Modified: libcxx/trunk/test/std/experimental/filesystem/class.rec.dir.itr/rec.dir.itr.members/increment.pass.cpp
URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/experimental/filesystem/class.rec.dir.itr/rec.dir.itr.members/increment.pass.cpp?rev=328476&r1=328475&r2=328476&view=diff
==============================================================================
--- libcxx/trunk/test/std/experimental/filesystem/class.rec.dir.itr/rec.dir.itr.members/increment.pass.cpp (original)
+++ libcxx/trunk/test/std/experimental/filesystem/class.rec.dir.itr/rec.dir.itr.members/increment.pass.cpp Sun Mar 25 23:23:55 2018
@@ -255,8 +255,8 @@ TEST_CASE(test_PR35078)
 
     // Change the permissions so we can no longer iterate
     permissions(permDeniedDir,
-                perms::remove_perms|perms::group_exec
-               |perms::owner_exec|perms::others_exec);
+                perms::group_exec|perms::owner_exec|perms::others_exec,
+                perm_options::remove);
 
     const std::error_code eacess_ec =
         std::make_error_code(std::errc::permission_denied);
@@ -329,8 +329,8 @@ TEST_CASE(test_PR35078_with_symlink)
 
     // Change the permissions so we can no longer iterate
     permissions(permDeniedDir,
-                perms::remove_perms|perms::group_exec
-               |perms::owner_exec|perms::others_exec);
+                perms::group_exec|perms::owner_exec|perms::others_exec,
+                perm_options::remove);
 
     const std::error_code eacess_ec =
         std::make_error_code(std::errc::permission_denied);
@@ -413,8 +413,8 @@ TEST_CASE(test_PR35078_with_symlink_file
 
     // Change the permissions so we can no longer iterate
     permissions(permDeniedDir,
-                perms::remove_perms|perms::group_exec
-               |perms::owner_exec|perms::others_exec);
+                perms::group_exec|perms::owner_exec|perms::others_exec,
+                perm_options::remove);
 
     const std::error_code eacess_ec =
         std::make_error_code(std::errc::permission_denied);

Modified: libcxx/trunk/test/std/experimental/filesystem/fs.enum/enum.perms.pass.cpp
URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/experimental/filesystem/fs.enum/enum.perms.pass.cpp?rev=328476&r1=328475&r2=328476&view=diff
==============================================================================
--- libcxx/trunk/test/std/experimental/filesystem/fs.enum/enum.perms.pass.cpp (original)
+++ libcxx/trunk/test/std/experimental/filesystem/fs.enum/enum.perms.pass.cpp Sun Mar 25 23:23:55 2018
@@ -59,9 +59,6 @@ int main() {
         E::set_gid      == ME(02000) &&
         E::sticky_bit   == ME(01000) &&
         E::mask         == ME(07777) &&
-        E::unknown      == ME(0xFFFF) &&
-        E::add_perms        == ME(0x10000) &&
-        E::remove_perms     == ME(0x20000) &&
-        E::symlink_nofollow == ME(0x40000),
+        E::unknown      == ME(0xFFFF),
         "Expected enumeration values do not match");
 }

Modified: libcxx/trunk/test/std/experimental/filesystem/fs.op.funcs/fs.op.create_directory/create_directory_with_attributes.pass.cpp
URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/experimental/filesystem/fs.op.funcs/fs.op.create_directory/create_directory_with_attributes.pass.cpp?rev=328476&r1=328475&r2=328476&view=diff
==============================================================================
--- libcxx/trunk/test/std/experimental/filesystem/fs.op.funcs/fs.op.create_directory/create_directory_with_attributes.pass.cpp (original)
+++ libcxx/trunk/test/std/experimental/filesystem/fs.op.funcs/fs.op.create_directory/create_directory_with_attributes.pass.cpp Sun Mar 25 23:23:55 2018
@@ -60,7 +60,7 @@ TEST_CASE(create_directory_one_level)
 {
     scoped_test_env env;
     // Remove setgid which mkdir would inherit
-    permissions(env.test_root, perms::remove_perms | perms::set_gid);
+    permissions(env.test_root, perms::set_gid, perm_options::remove);
 
     const path dir = env.make_env_path("dir1");
     const path attr_dir = env.create_dir("dir2");

Modified: libcxx/trunk/test/std/experimental/filesystem/fs.op.funcs/fs.op.permissions/permissions.pass.cpp
URL: http://llvm.org/viewvc/llvm-project/libcxx/trunk/test/std/experimental/filesystem/fs.op.funcs/fs.op.permissions/permissions.pass.cpp?rev=328476&r1=328475&r2=328476&view=diff
==============================================================================
--- libcxx/trunk/test/std/experimental/filesystem/fs.op.funcs/fs.op.permissions/permissions.pass.cpp (original)
+++ libcxx/trunk/test/std/experimental/filesystem/fs.op.funcs/fs.op.permissions/permissions.pass.cpp Sun Mar 25 23:23:55 2018
@@ -11,8 +11,11 @@
 
 // <experimental/filesystem>
 
-// void permissions(const path& p, perms prms);
+// void permissions(const path& p, perms prms,
+//                  perm_options opts = perm_options::replace);
 // void permissions(const path& p, perms prms, std::error_code& ec) noexcept;
+// void permissions(const path& p, perms prms, perm_options opts, std::error_code);
+
 
 
 #include "filesystem_include.hpp"
@@ -30,17 +33,19 @@ TEST_SUITE(filesystem_permissions_test_s
 TEST_CASE(test_signatures)
 {
     const path p; ((void)p);
-    const perms opts{}; ((void)opts);
+    const perms pr{}; ((void)pr);
+    const perm_options opts{}; ((void)opts);
     std::error_code ec; ((void)ec);
-    ASSERT_NOT_NOEXCEPT(fs::permissions(p, opts));
-    // Not noexcept because of narrow contract
-    LIBCPP_ONLY(
-        ASSERT_NOT_NOEXCEPT(fs::permissions(p, opts, ec)));
+    ASSERT_NOT_NOEXCEPT(fs::permissions(p, pr));
+    ASSERT_NOT_NOEXCEPT(fs::permissions(p, pr, opts));
+    ASSERT_NOEXCEPT(fs::permissions(p, pr, ec));
+    ASSERT_NOT_NOEXCEPT(fs::permissions(p, pr, opts, ec));
 }
 
 TEST_CASE(test_error_reporting)
 {
-    auto checkThrow = [](path const& f, fs::perms opts, const std::error_code& ec)
+    auto checkThrow = [](path const& f, fs::perms opts,
+                         const std::error_code& ec)
     {
 #ifndef TEST_HAS_NO_EXCEPTIONS
         try {
@@ -61,15 +66,17 @@ TEST_CASE(test_error_reporting)
     const path dne = env.make_env_path("dne");
     const path dne_sym = env.create_symlink(dne, "dne_sym");
     { // !exists
-        std::error_code ec;
+        std::error_code ec = GetTestEC();
         fs::permissions(dne, fs::perms{}, ec);
         TEST_REQUIRE(ec);
+        TEST_CHECK(ec != GetTestEC());
         TEST_CHECK(checkThrow(dne, fs::perms{}, ec));
     }
     {
-        std::error_code ec;
+        std::error_code ec = GetTestEC();
         fs::permissions(dne_sym, fs::perms{}, ec);
         TEST_REQUIRE(ec);
+        TEST_CHECK(ec != GetTestEC());
         TEST_CHECK(checkThrow(dne_sym, fs::perms{}, ec));
     }
 }
@@ -81,42 +88,51 @@ TEST_CASE(basic_permissions_test)
     const path dir = env.create_dir("dir1");
     const path file_for_sym = env.create_file("file2", 42);
     const path sym = env.create_symlink(file_for_sym, "sym");
-    const perms AP = perms::add_perms;
-    const perms RP = perms::remove_perms;
-    const perms NF = perms::symlink_nofollow;
+    const perm_options AP = perm_options::add;
+    const perm_options RP = perm_options::remove;
+    const perm_options NF = perm_options::nofollow;
     struct TestCase {
       path p;
       perms set_perms;
       perms expected;
+      perm_options opts = perm_options::replace;
     } cases[] = {
         // test file
         {file, perms::none, perms::none},
         {file, perms::owner_all, perms::owner_all},
-        {file, perms::group_all | AP, perms::owner_all | perms::group_all},
-        {file, perms::group_all | RP, perms::owner_all},
+        {file, perms::group_all, perms::owner_all | perms::group_all, AP},
+        {file, perms::group_all, perms::owner_all, RP},
         // test directory
         {dir, perms::none, perms::none},
         {dir, perms::owner_all, perms::owner_all},
-        {dir, perms::group_all | AP, perms::owner_all | perms::group_all},
-        {dir, perms::group_all | RP, perms::owner_all},
+        {dir, perms::group_all, perms::owner_all | perms::group_all, AP},
+        {dir, perms::group_all, perms::owner_all, RP},
         // test symlink without symlink_nofollow
         {sym, perms::none, perms::none},
         {sym, perms::owner_all, perms::owner_all},
-        {sym, perms::group_all | AP, perms::owner_all | perms::group_all},
-        {sym, perms::group_all | RP , perms::owner_all},
+        {sym, perms::group_all, perms::owner_all | perms::group_all, AP},
+        {sym, perms::group_all, perms::owner_all, RP},
         // test non-symlink with symlink_nofollow. The last test on file/dir
         // will have set their permissions to perms::owner_all
-        {file, perms::group_all | AP | NF, perms::owner_all | perms::group_all},
-        {dir,  perms::group_all | AP | NF, perms::owner_all | perms::group_all}
+        {file, perms::group_all, perms::owner_all | perms::group_all, AP | NF},
+        {dir,  perms::group_all, perms::owner_all | perms::group_all, AP | NF}
     };
     for (auto const& TC : cases) {
         TEST_CHECK(status(TC.p).permissions() != TC.expected);
-        // Set the error code to ensure it's cleared.
-        std::error_code ec = std::make_error_code(std::errc::bad_address);
-        permissions(TC.p, TC.set_perms, ec);
-        TEST_CHECK(!ec);
-        auto pp = status(TC.p).permissions();
-        TEST_CHECK(pp == TC.expected);
+        {
+          std::error_code ec = GetTestEC();
+          permissions(TC.p, TC.set_perms, TC.opts, ec);
+          TEST_CHECK(!ec);
+          auto pp = status(TC.p).permissions();
+          TEST_CHECK(pp == TC.expected);
+        }
+        if (TC.opts == perm_options::replace) {
+          std::error_code ec = GetTestEC();
+          permissions(TC.p, TC.set_perms, ec);
+          TEST_CHECK(!ec);
+          auto pp = status(TC.p).permissions();
+          TEST_CHECK(pp == TC.expected);
+        }
     }
 }
 
@@ -130,10 +146,11 @@ TEST_CASE(test_no_resolve_symlink_on_sym
     struct TestCase {
         perms set_perms;
         perms expected; // only expected on platform that support symlink perms.
+        perm_options opts = perm_options::replace;
     } cases[] = {
         {perms::owner_all, perms::owner_all},
-        {perms::group_all | perms::add_perms, perms::owner_all | perms::group_all},
-        {perms::owner_all | perms::remove_perms, perms::group_all},
+        {perms::group_all, perms::owner_all | perms::group_all, perm_options::add},
+        {perms::owner_all, perms::group_all, perm_options::remove},
     };
     for (auto const& TC : cases) {
 #if defined(__APPLE__) || defined(__FreeBSD__)
@@ -148,8 +165,8 @@ TEST_CASE(test_no_resolve_symlink_on_sym
         const auto expected_link_perms = symlink_status(sym).permissions();
         std::error_code expected_ec = std::make_error_code(std::errc::operation_not_supported);
 #endif
-        std::error_code ec = std::make_error_code(std::errc::bad_address);
-        permissions(sym, TC.set_perms | perms::symlink_nofollow, ec);
+        std::error_code ec = GetTestEC();
+        permissions(sym, TC.set_perms, TC.opts | perm_options::nofollow, ec);
         TEST_CHECK(ec == expected_ec);
         TEST_CHECK(status(file).permissions() == file_perms);
         TEST_CHECK(symlink_status(sym).permissions() == expected_link_perms);




More information about the cfe-commits mailing list