[libcxx-commits] [PATCH] D101728: [libcxx] [test] Fix filesystem permission tests for windows

Arthur O'Dwyer via Phabricator via libcxx-commits libcxx-commits at lists.llvm.org
Wed May 5 13:39:21 PDT 2021

Quuxplusone accepted this revision as: Quuxplusone.
Quuxplusone added a comment.

LGTM % comment.

Comment at: libcxx/test/support/filesystem_test_helper.h:621
+inline bool PermsEq(fs::perms Actual, fs::perms Expected) {
+  return Actual == PermsNormalize(Expected);
mstorsjo wrote:
> I guess it can be argued that we should normalize both operands, as it's not evident to the callers about which parameter is which. But by only normalizing the expectation, we keep the test a bit stronger. But I can be convinced otherwise too.
If you're only normalizing one side, I'd name the function `PermsNormalizeExpected` (since you could perhaps have chosen to implement a `PermsNormalizeActual` instead). However, it's also a function with only one caller, so I'd actually inline it. I'd say
inline bool PermsEq(fs::perms Actual, fs::perms Expected) {
#ifdef _WIN32
  // On Windows, fs::perms maps down to one bit stored in the filesystem,
  // a boolean readonly flag.
  // Normalize the expected permissions to the format that is returned:
  // all files are read+exec for all users; writable files are writable for all users.
  Expected |= fs::perms::owner_read | fs::perms::group_read | fs::perms::others_read;
  Expected |= fs::perms::owner_exec | fs::perms::group_exec | fs::perms::others_exec;
  if (Expected & (fs::perms::owner_write | fs::perms::group_write | fs::perms::others_write))
    Expected |= fs::perms::owner_write | fs::perms::group_write | fs::perms::others_write;
  return Actual == Expected;

  rG LLVM Github Monorepo



More information about the libcxx-commits mailing list