Thread premissions through sys::fs::create_director{y|ies}

[Justin Bogner]

Frédéric Riss <friss at apple.com> writes:
> The attached pretty trivial patch thread a permissions argument
> through sys::fs::create_directories.
> I’m sending it for review regardless of the trivialness for 2 reasons:
>  - I have no idea how to test that. The passed permission are still
>    subject to umask, thus testing the file_status of a created directory
>    in a unittest doesn’t seem possible. Ideas welcome.

Can't you just call umask(2) in the test?

>  - I suppose Windows doesn’t care about permissions, thus I just
>    ignored the argument. Is that right?
>
> commit be795a04ef70e7832b26a46bbfe82fd618ebc701
> Author: Frederic Riss <friss at apple.com>
> Date:   Tue Aug 4 22:48:27 2015 -0700
>
>     Thread premissions through sys::fs::create_director{y|ies}
>
> diff --git a/include/llvm/Support/FileSystem.h b/include/llvm/Support/FileSystem.h
> index a736c32..36c669a 100644
> --- a/include/llvm/Support/FileSystem.h
> +++ b/include/llvm/Support/FileSystem.h
> @@ -285,7 +285,8 @@ std::error_code make_absolute(SmallVectorImpl<char> &path);
>  ///          specific error_code. If IgnoreExisting is false, also returns
>  ///          error if the directory already existed.
>  std::error_code create_directories(const Twine &path,
> -                                   bool IgnoreExisting = true);
> +                                   bool IgnoreExisting = true,
> +                                   perms Perms = owner_all | group_all);
>  
>  /// @brief Create the directory in path.
>  ///
> @@ -293,7 +294,8 @@ std::error_code create_directories(const Twine &path,
>  /// @returns errc::success if is_directory(path), otherwise a platform
>  ///          specific error_code. If IgnoreExisting is false, also returns
>  ///          error if the directory already existed.
> -std::error_code create_directory(const Twine &path, bool IgnoreExisting = true);
> +std::error_code create_directory(const Twine &path, bool IgnoreExisting = true,
> +                                 perms Perms = owner_all | group_all);
>  
>  /// @brief Create a link from \a from to \a to.
>  ///
> diff --git a/lib/Support/Path.cpp b/lib/Support/Path.cpp
> index 985cdbf..54daf24 100644
> --- a/lib/Support/Path.cpp
> +++ b/lib/Support/Path.cpp
> @@ -785,12 +785,13 @@ std::error_code make_absolute(SmallVectorImpl<char> &path) {
>                     "occurred above!");
>  }
>  
> -std::error_code create_directories(const Twine &Path, bool IgnoreExisting) {
> +std::error_code create_directories(const Twine &Path, bool IgnoreExisting,
> +                                   perms Perms) {
>    SmallString<128> PathStorage;
>    StringRef P = Path.toStringRef(PathStorage);
>  
>    // Be optimistic and try to create the directory
> -  std::error_code EC = create_directory(P, IgnoreExisting);
> +  std::error_code EC = create_directory(P, IgnoreExisting, Perms);
>    // If we succeeded, or had any error other than the parent not existing, just
>    // return it.
>    if (EC != errc::no_such_file_or_directory)
> @@ -802,10 +803,10 @@ std::error_code create_directories(const Twine &Path, bool IgnoreExisting) {
>    if (Parent.empty())
>      return EC;
>  
> -  if ((EC = create_directories(Parent)))
> +  if ((EC = create_directories(Parent, IgnoreExisting, Perms)))
>        return EC;
>  
> -  return create_directory(P, IgnoreExisting);
> +  return create_directory(P, IgnoreExisting, Perms);
>  }
>  
>  std::error_code copy_file(const Twine &From, const Twine &To) {
> diff --git a/lib/Support/Unix/Path.inc b/lib/Support/Unix/Path.inc
> index 973d010..a77efcc 100644
> --- a/lib/Support/Unix/Path.inc
> +++ b/lib/Support/Unix/Path.inc
> @@ -219,11 +219,12 @@ std::error_code current_path(SmallVectorImpl<char> &result) {
>    return std::error_code();
>  }
>  
> -std::error_code create_directory(const Twine &path, bool IgnoreExisting) {
> +std::error_code create_directory(const Twine &path, bool IgnoreExisting,
> +                                 perms Perms) {
>    SmallString<128> path_storage;
>    StringRef p = path.toNullTerminatedStringRef(path_storage);
>  
> -  if (::mkdir(p.begin(), S_IRWXU | S_IRWXG) == -1) {
> +  if (::mkdir(p.begin(), Perms) == -1) {
>      if (errno != EEXIST || !IgnoreExisting)
>        return std::error_code(errno, std::generic_category());
>    }
> diff --git a/lib/Support/Windows/Path.inc b/lib/Support/Windows/Path.inc
> index 72da7c5..3033149 100644
> --- a/lib/Support/Windows/Path.inc
> +++ b/lib/Support/Windows/Path.inc
> @@ -182,7 +182,8 @@ std::error_code current_path(SmallVectorImpl<char> &result) {
>    return UTF16ToUTF8(cur_path.begin(), cur_path.size(), result);
>  }
>  
> -std::error_code create_directory(const Twine &path, bool IgnoreExisting) {
> +std::error_code create_directory(const Twine &path, bool IgnoreExisting,
> +                                 perms Perms) {

Better to omit the unused parameter name, ie, `perms /*Perms*/`. This
way -Wunused-parameter will be happy with it.

>    SmallVector<wchar_t, 128> path_utf16;
>  
>    if (std::error_code ec = widenPath(path, path_utf16))
>  
>
> Fred
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits