From ca84d3dd2e9e27645d579ac705338c6657fc693b Mon Sep 17 00:00:00 2001 From: ljusten Date: Thu, 17 Nov 2022 14:01:59 +0100 Subject: [PATCH] [cdc_fuse_fs] Fix various issues (#6) Fixes a couple of issues with the FUSE: - Creates the mount directory if it does not exist. This assumes the mount dir to be the last arg. Ideally, we'd parse the command line and then create the directory, but unfortunately fuse_parse_cmdline already verifies that the dir exists. - Expands the cache_dir (e.g. ~). - Fixes a compile issue in manifest_iterator. --- cdc_fuse_fs/main.cc | 38 ++++++++++++++++++++++++++++++++--- common/BUILD | 8 +++++--- common/path.cc | 15 ++++++++++++-- common/path.h | 11 ++++++---- common/path_test.cc | 22 +++++++++++++++----- data_store/disk_data_store.cc | 3 +++ 6 files changed, 80 insertions(+), 17 deletions(-) diff --git a/cdc_fuse_fs/main.cc b/cdc_fuse_fs/main.cc index 362afec..ef23e67 100644 --- a/cdc_fuse_fs/main.cc +++ b/cdc_fuse_fs/main.cc @@ -73,6 +73,31 @@ bool IsUpToDate(const std::string& components_arg) { return true; } +absl::Status CreateMountDir(const std::vector& args) { + // Assume the mount dir is the last argument. + size_t argc = args.size(); + if (argc < 2 || !args[argc - 1]) { + return absl::InvalidArgumentError( + "The last argument must be the mount directory"); + } + std::string mount_dir = args[argc - 1]; + if (mount_dir[0] == '-') { + return absl::InvalidArgumentError(absl::StrFormat( + "The last argument must be the mount directory, but is '%s'", + mount_dir)); + } + + // Expand ~ etc. + RETURN_IF_ERROR(cdc_ft::path::ExpandPathVariables(&mount_dir), + "Failed to expand mount directory '%s'", mount_dir); + + // Create expanded directory. + RETURN_IF_ERROR(cdc_ft::path::CreateDirRec(mount_dir), + "Failed to create directory '%s'", mount_dir); + + return absl::OkStatus(); +} + } // namespace } // namespace cdc_ft @@ -105,7 +130,7 @@ ABSL_FLAG(uint32_t, access_idle_timeout, cdc_ft::DataProvider::kAccessIdleSec, static_assert(static_cast(absl::StatusCode::kOk) == 0, "kOk != 0"); -// Usage: cdc_fuse_fs -- mount_dir [-d|-s|..] +// Usage: cdc_fuse_fs -- [-d|-s|.. mount_dir] // Any args after -- are FUSE args, search third_party/fuse for FUSE_OPT_KEY or // FUSE_LIB_OPT (there doesn't seem to be a place where they're all described). int main(int argc, char* argv[]) { @@ -137,9 +162,16 @@ int main(int argc, char* argv[]) { printf("%s\n", cdc_ft::kFuseUpToDate); fflush(stdout); + // Create mount dir if it doesn't exist yet. + absl::Status status = cdc_ft::CreateMountDir(mount_args); + if (!status.ok()) { + LOG_ERROR("Failed to create mount directory: %s", status.ToString()); + return 1; + } + // Create fs. The rest of the flags are mount flags, so pass them along. - absl::Status status = cdc_ft::cdc_fuse_fs::Initialize( - static_cast(mount_args.size()), mount_args.data()); + status = cdc_ft::cdc_fuse_fs::Initialize(static_cast(mount_args.size()), + mount_args.data()); if (!status.ok()) { LOG_ERROR("Failed to initialize file system: %s", status.ToString()); return static_cast(status.code()); diff --git a/common/BUILD b/common/BUILD index 76657c0..800acd2 100644 --- a/common/BUILD +++ b/common/BUILD @@ -35,16 +35,18 @@ cc_library( # Additional warnings from @com_github_dirent "/wd4505", # unreferenced function with internal linkage has been removed ], - "//conditions:default": ["/wd4505"], + "//conditions:default": [], }), deps = [ ":path", ":platform", - "@com_github_dirent//:dirent", "@com_google_absl//absl/status", "@com_google_absl//absl/strings", "@com_google_absl//absl/strings:str_format", - ], + ] + select({ + "//tools:windows": ["@com_github_dirent//:dirent"], + "//conditions:default": [], + }), ) cc_test( diff --git a/common/path.cc b/common/path.cc index e5b676f..b5d7e87 100644 --- a/common/path.cc +++ b/common/path.cc @@ -37,6 +37,7 @@ #include // putenv #include // readlink #include // struct utimbuf +#include #define __stat64 stat64 #define _chmod chmod #endif @@ -198,8 +199,8 @@ absl::Status GetKnownFolderPath(FolderId folder_id, std::string* path) { } #endif +absl::Status ExpandPathVariables(std::string* path) { #if PLATFORM_WINDOWS -absl::Status ExpandEnvironmentPathVariables(std::string* path) { std::wstring wchar_path = Util::Utf8ToWideStr(*path); DWORD size = ::ExpandEnvironmentStrings(wchar_path.c_str(), nullptr, 0); @@ -217,8 +218,18 @@ absl::Status ExpandEnvironmentPathVariables(std::string* path) { wchar_expanded.pop_back(); *path = Util::WideToUtf8Str(wchar_expanded); return absl::OkStatus(); -} +#else + wordexp_t res; + wordexp(path->c_str(), &res, 0); + if (res.we_wordc > 1) { + return absl::InvalidArgumentError( + "Path expands to multiple results (did you use * etc. ?"); + } + *path = res.we_wordv[0]; + wordfree(&res); + return absl::OkStatus(); #endif +} absl::Status GetEnv(const std::string& name, std::string* value) { value->clear(); diff --git a/common/path.h b/common/path.h index 73fb9bf..8901277 100644 --- a/common/path.h +++ b/common/path.h @@ -99,12 +99,15 @@ enum class FolderId { // Returns the Windows known folder path for the given |folder_id|. absl::Status GetKnownFolderPath(FolderId folder_id, std::string* path); - -// Expands environment path variables like %APPDATA%. Variables are matched -// case invariantly. Unknown environment variables are not changed. -absl::Status ExpandEnvironmentPathVariables(std::string* path); #endif +// Expands environment path variables like %APPDATA% on Windows or ~ on Linux. +// On Windows, variables are matched case invariantly. Unknown environment +// variables are not changed. +// On Linux, performs a shell-like expansion. Returns an error if multiple +// results would be returned, e.g. from *.txt. +absl::Status ExpandPathVariables(std::string* path); + // Returns the environment variable with given |name| in |value|. // Returns a NotFound error and sets |value| to an empty string if the variable // does not exist. diff --git a/common/path_test.cc b/common/path_test.cc index 7ea49ae..e0a08e0 100644 --- a/common/path_test.cc +++ b/common/path_test.cc @@ -199,22 +199,34 @@ TEST_F(PathTest, GetKnownFolderPath) { } #endif +TEST_F(PathTest, ExpandPathVariables) { #if PLATFORM_WINDOWS -TEST_F(PathTest, ExpandEnvironmentPathVariables) { std::string path = u8"%userPROfile%\\fOo\U0001F964"; - EXPECT_OK(path::ExpandEnvironmentPathVariables(&path)); + EXPECT_OK(path::ExpandPathVariables(&path)); EXPECT_TRUE(absl::StartsWith(path, "C:\\Users\\")) << path; EXPECT_TRUE(absl::EndsWith(path, u8"\\fOo\U0001F964")) << path; path = "%ProgramFiles(x86)%\\Foo"; - EXPECT_OK(path::ExpandEnvironmentPathVariables(&path)); + EXPECT_OK(path::ExpandPathVariables(&path)); EXPECT_EQ(path, "C:\\Program Files (x86)\\Foo"); path = "%unrelated%\\fOo"; - EXPECT_OK(path::ExpandEnvironmentPathVariables(&path)); + EXPECT_OK(path::ExpandPathVariables(&path)); EXPECT_EQ(path, "%unrelated%\\fOo") << path; -} +#else + std::string path = u8"fooU0001F964"; + EXPECT_OK(path::ExpandPathVariables(&path)); + EXPECT_EQ(path, u8"fooU0001F964"); + + path = "~/foo"; + EXPECT_OK(path::ExpandPathVariables(&path)); + EXPECT_TRUE(absl::StrContains(path, "home")) << path; + + path = "*"; + EXPECT_ERROR_MSG(InvalidArgument, "multiple results", + path::ExpandPathVariables(&path)); #endif +} TEST_F(PathTest, GetEnv_DoesNotExist) { std::string value; diff --git a/data_store/disk_data_store.cc b/data_store/disk_data_store.cc index 531092b..3daa33b 100644 --- a/data_store/disk_data_store.cc +++ b/data_store/disk_data_store.cc @@ -84,6 +84,9 @@ DiskDataStore::DiskDataStore(unsigned int depth, std::string cache_root_dir, absl::StatusOr> DiskDataStore::Create( unsigned int depth, std::string cache_root_dir, bool create_dirs, SystemClock* clock) { + // Resolve e.g. ~. + RETURN_IF_ERROR(path::ExpandPathVariables(&cache_root_dir), + "Failed to expand cache dir '%s'", cache_root_dir); std::unique_ptr store = absl::WrapUnique( new DiskDataStore(depth, std::move(cache_root_dir), create_dirs, clock)); if (create_dirs) {