Author: ericwf Date: Tue Jun 21 17:42:42 2016 New Revision: 273328 URL: http://llvm.org/viewvc/llvm-project?rev=273328&view=rev Log: Implement LWG issue 2720. Replace perms::resolve_symlinks with perms::symlink_nofollow.
This changes how filesystem::permissions(p, perms) handles symlinks. Previously symlinks were not resolved by default instead only getting resolved when "perms::resolve_symlinks" was used. After this change symlinks are resolved by default and perms::symlink_nofollow must be given to change this. This issue has not yet been moved to Ready status, and I will revert if it doesn't get moved at the current meeting. However I feel confident that it will and it's nice to have implementations when moving issues. Modified: libcxx/trunk/include/experimental/filesystem libcxx/trunk/src/experimental/filesystem/operations.cpp libcxx/trunk/test/std/experimental/filesystem/fs.enum/enum.perms.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=273328&r1=273327&r2=273328&view=diff ============================================================================== --- libcxx/trunk/include/experimental/filesystem (original) +++ libcxx/trunk/include/experimental/filesystem Tue Jun 21 17:42:42 2016 @@ -292,7 +292,7 @@ enum class _LIBCPP_TYPE_VIS perms : unsi add_perms = 0x10000, remove_perms = 0x20000, - resolve_symlinks = 0x40000 + symlink_nofollow = 0x40000 }; _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=273328&r1=273327&r2=273328&view=diff ============================================================================== --- libcxx/trunk/src/experimental/filesystem/operations.cpp (original) +++ libcxx/trunk/src/experimental/filesystem/operations.cpp Tue Jun 21 17:42:42 2016 @@ -594,7 +594,7 @@ void __last_write_time(const path& p, fi void __permissions(const path& p, perms prms, std::error_code *ec) { - const bool resolve_symlinks = bool(perms::resolve_symlinks & prms); + 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); @@ -605,6 +605,8 @@ void __permissions(const path& p, perms file_status st = detail::posix_lstat(p, &m_ec); if (m_ec) return set_or_throw(m_ec, ec, "permissions", p); + // AT_SYMLINK_NOFOLLOW can only be used on symlinks, using it on a regular + // file will cause fchmodat to report an error on some systems. const bool set_sym_perms = is_symlink(st) && !resolve_symlinks; if ((resolve_symlinks && is_symlink(st)) && (add_perms || remove_perms)) { @@ -622,14 +624,16 @@ void __permissions(const path& p, perms # if defined(AT_SYMLINK_NOFOLLOW) && defined(AT_FDCWD) const int flags = set_sym_perms ? AT_SYMLINK_NOFOLLOW : 0; if (::fchmodat(AT_FDCWD, p.c_str(), real_perms, flags) == -1) { + return set_or_throw(ec, "permissions", p); + } # else if (set_sym_perms) return set_or_throw(make_error_code(errc::operation_not_supported), ec, "permissions", p); if (::chmod(p.c_str(), real_perms) == -1) { -# endif return set_or_throw(ec, "permissions", p); } +# endif if (ec) ec->clear(); } 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=273328&r1=273327&r2=273328&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 Tue Jun 21 17:42:42 2016 @@ -63,6 +63,6 @@ int main() { E::unknown == ME(0xFFFF) && E::add_perms == ME(0x10000) && E::remove_perms == ME(0x20000) && - E::resolve_symlinks == ME(0x40000), + E::symlink_nofollow == ME(0x40000), "Expected enumeration values do not match"); } 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=273328&r1=273327&r2=273328&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 Tue Jun 21 17:42:42 2016 @@ -67,9 +67,9 @@ TEST_CASE(test_error_reporting) } { std::error_code ec; - fs::permissions(dne_sym, perms::resolve_symlinks, ec); + fs::permissions(dne_sym, fs::perms{}, ec); TEST_REQUIRE(ec); - TEST_CHECK(checkThrow(dne_sym, perms::resolve_symlinks, ec)); + TEST_CHECK(checkThrow(dne_sym, fs::perms{}, ec)); } } @@ -82,7 +82,7 @@ TEST_CASE(basic_permissions_test) const path sym = env.create_symlink(file_for_sym, "sym"); const perms AP = perms::add_perms; const perms RP = perms::remove_perms; - const perms RS = perms::resolve_symlinks; + const perms NF = perms::symlink_nofollow; struct TestCase { path p; perms set_perms; @@ -98,11 +98,15 @@ TEST_CASE(basic_permissions_test) {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}, - // test symlink with resolve symlinks on symlink - {sym, perms::none | RS, perms::none}, - {sym, perms::owner_all | RS, perms::owner_all}, - {sym, perms::group_all | AP | RS, perms::owner_all | perms::group_all}, - {sym, perms::group_all | RP | RS, perms::owner_all} + // 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}, + // 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} }; for (auto const& TC : cases) { TEST_CHECK(status(TC.p).permissions() != TC.expected); @@ -144,7 +148,7 @@ TEST_CASE(test_no_resolve_symlink_on_sym 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, ec); + permissions(sym, TC.set_perms | perms::symlink_nofollow, ec); TEST_CHECK(ec == expected_ec); TEST_CHECK(status(file).permissions() == file_perms); TEST_CHECK(symlink_status(sym).permissions() == expected_link_perms); _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits