diff --git a/adb/adb_utils.cpp b/adb/adb_utils.cpp index 7afa6169c..7058acb38 100644 --- a/adb/adb_utils.cpp +++ b/adb/adb_utils.cpp @@ -19,16 +19,15 @@ #include "adb_utils.h" #include "adb_unique_fd.h" -#include #include #include #include #include #include -#include #include +#include #include #include #include @@ -100,52 +99,6 @@ std::string escape_arg(const std::string& s) { return result; } -std::string adb_basename(const std::string& path) { - static std::mutex& basename_lock = *new std::mutex(); - - // Copy path because basename may modify the string passed in. - std::string result(path); - - // Use lock because basename() may write to a process global and return a - // pointer to that. Note that this locking strategy only works if all other - // callers to basename in the process also grab this same lock. - std::lock_guard lock(basename_lock); - - // Note that if std::string uses copy-on-write strings, &str[0] will cause - // the copy to be made, so there is no chance of us accidentally writing to - // the storage for 'path'. - char* name = basename(&result[0]); - - // In case dirname returned a pointer to a process global, copy that string - // before leaving the lock. - result.assign(name); - - return result; -} - -std::string adb_dirname(const std::string& path) { - static std::mutex& dirname_lock = *new std::mutex(); - - // Copy path because dirname may modify the string passed in. - std::string result(path); - - // Use lock because dirname() may write to a process global and return a - // pointer to that. Note that this locking strategy only works if all other - // callers to dirname in the process also grab this same lock. - std::lock_guard lock(dirname_lock); - - // Note that if std::string uses copy-on-write strings, &str[0] will cause - // the copy to be made, so there is no chance of us accidentally writing to - // the storage for 'path'. - char* parent = dirname(&result[0]); - - // In case dirname returned a pointer to a process global, copy that string - // before leaving the lock. - result.assign(parent); - - return result; -} - // Given a relative or absolute filepath, create the directory hierarchy // as needed. Returns true if the hierarchy is/was setup. bool mkdirs(const std::string& path) { @@ -171,7 +124,7 @@ bool mkdirs(const std::string& path) { return true; } - const std::string parent(adb_dirname(path)); + const std::string parent(android::base::Dirname(path)); // If dirname returned the same path as what we passed in, don't go recursive. // This can happen on Windows when walking up the directory hierarchy and not diff --git a/adb/adb_utils.h b/adb/adb_utils.h index 2b5903455..e0ad10380 100644 --- a/adb/adb_utils.h +++ b/adb/adb_utils.h @@ -28,11 +28,6 @@ void close_stdin(); bool getcwd(std::string* cwd); bool directory_exists(const std::string& path); -// Like the regular basename and dirname, but thread-safe on all -// platforms and capable of correctly handling exotic Windows paths. -std::string adb_basename(const std::string& path); -std::string adb_dirname(const std::string& path); - // Return the user's home directory. std::string adb_get_homedir_path(); diff --git a/adb/adb_utils_test.cpp b/adb/adb_utils_test.cpp index 4cac48508..a3bc44560 100644 --- a/adb/adb_utils_test.cpp +++ b/adb/adb_utils_test.cpp @@ -109,18 +109,6 @@ TEST(adb_utils, escape_arg) { ASSERT_EQ(R"('abc)')", escape_arg("abc)")); } -TEST(adb_utils, adb_basename) { - EXPECT_EQ("sh", adb_basename("/system/bin/sh")); - EXPECT_EQ("sh", adb_basename("sh")); - EXPECT_EQ("sh", adb_basename("/system/bin/sh/")); -} - -TEST(adb_utils, adb_dirname) { - EXPECT_EQ("/system/bin", adb_dirname("/system/bin/sh")); - EXPECT_EQ(".", adb_dirname("sh")); - EXPECT_EQ("/system/bin", adb_dirname("/system/bin/sh/")); -} - void test_mkdirs(const std::string& basepath) { // Test creating a directory hierarchy. ASSERT_TRUE(mkdirs(basepath)); diff --git a/adb/bugreport.cpp b/adb/bugreport.cpp index b7e76a62a..a5c312bbe 100644 --- a/adb/bugreport.cpp +++ b/adb/bugreport.cpp @@ -21,6 +21,7 @@ #include #include +#include #include #include "sysdeps.h" @@ -113,14 +114,14 @@ class BugreportStandardStreamsCallback : public StandardStreamsCallbackInterface private: void SetLineMessage(const std::string& action) { - line_message_ = action + " " + adb_basename(dest_file_); + line_message_ = action + " " + android::base::Basename(dest_file_); } void SetSrcFile(const std::string path) { src_file_ = path; if (!dest_dir_.empty()) { // Only uses device-provided name when user passed a directory. - dest_file_ = adb_basename(path); + dest_file_ = android::base::Basename(path); SetLineMessage("generating"); } } diff --git a/adb/commandline.cpp b/adb/commandline.cpp index 5a2206f6d..3de7be670 100644 --- a/adb/commandline.cpp +++ b/adb/commandline.cpp @@ -2115,7 +2115,8 @@ static int install_multiple_app(TransportType transport, const char* serial, int std::string cmd = android::base::StringPrintf( "%s install-write -S %" PRIu64 " %d %d_%s -", - install_cmd.c_str(), static_cast(sb.st_size), session_id, i, adb_basename(file).c_str()); + install_cmd.c_str(), static_cast(sb.st_size), session_id, i, + android::base::Basename(file).c_str()); int localFd = adb_open(file, O_RDONLY); if (localFd < 0) { @@ -2233,7 +2234,7 @@ static int install_app_legacy(TransportType transport, const char* serial, int a int result = -1; std::vector apk_file = {argv[last_apk]}; std::string apk_dest = android::base::StringPrintf( - where, adb_basename(argv[last_apk]).c_str()); + where, android::base::Basename(argv[last_apk]).c_str()); if (!do_sync_push(apk_file, apk_dest.c_str())) goto cleanup_apk; argv[last_apk] = apk_dest.c_str(); /* destination name, not source location */ result = pm_command(transport, serial, argc, argv); diff --git a/adb/file_sync_client.cpp b/adb/file_sync_client.cpp index 271943d31..22bd2f297 100644 --- a/adb/file_sync_client.cpp +++ b/adb/file_sync_client.cpp @@ -844,7 +844,8 @@ static bool local_build_list(SyncConnection& sc, std::vector* file_lis // TODO(b/25566053): Make pushing empty directories work. // TODO(b/25457350): We don't preserve permissions on directories. sc.Warning("skipping empty directory '%s'", lpath.c_str()); - copyinfo ci(adb_dirname(lpath), adb_dirname(rpath), adb_basename(lpath), S_IFDIR); + copyinfo ci(android::base::Dirname(lpath), android::base::Dirname(rpath), + android::base::Basename(lpath), S_IFDIR); ci.skip = true; file_list->push_back(ci); return true; @@ -977,7 +978,7 @@ bool do_sync_push(const std::vector& srcs, const char* dst) { if (dst_dir.back() != '/') { dst_dir.push_back('/'); } - dst_dir.append(adb_basename(src_path)); + dst_dir.append(android::base::Basename(src_path)); } success &= copy_local_dir_remote(sc, src_path, dst_dir.c_str(), false, false); @@ -995,7 +996,7 @@ bool do_sync_push(const std::vector& srcs, const char* dst) { if (path_holder.back() != '/') { path_holder.push_back('/'); } - path_holder += adb_basename(src_path); + path_holder += android::base::Basename(src_path); dst_path = path_holder.c_str(); } @@ -1015,7 +1016,8 @@ static bool remote_build_list(SyncConnection& sc, std::vector* file_li std::vector linklist; // Add an entry for the current directory to ensure it gets created before pulling its contents. - copyinfo ci(adb_dirname(lpath), adb_dirname(rpath), adb_basename(lpath), S_IFDIR); + copyinfo ci(android::base::Dirname(lpath), android::base::Dirname(rpath), + android::base::Basename(lpath), S_IFDIR); file_list->push_back(ci); // Put the files/dirs in rpath on the lists. @@ -1149,7 +1151,7 @@ bool do_sync_pull(const std::vector& srcs, const char* dst, if (srcs.size() == 1 && errno == ENOENT) { // However, its parent must exist. struct stat parent_st; - if (stat(adb_dirname(dst).c_str(), &parent_st) == -1) { + if (stat(android::base::Dirname(dst).c_str(), &parent_st) == -1) { sc.Error("cannot create file/directory '%s': %s", dst, strerror(errno)); return false; } @@ -1204,7 +1206,7 @@ bool do_sync_pull(const std::vector& srcs, const char* dst, if (!adb_is_separator(dst_dir.back())) { dst_dir.push_back(OS_PATH_SEPARATOR); } - dst_dir.append(adb_basename(src_path)); + dst_dir.append(android::base::Basename(src_path)); } success &= copy_remote_dir_local(sc, src_path, dst_dir.c_str(), copy_attrs); @@ -1220,7 +1222,7 @@ bool do_sync_pull(const std::vector& srcs, const char* dst, // really want to copy to local_dir + OS_PATH_SEPARATOR + // basename(remote). path_holder = android::base::StringPrintf("%s%c%s", dst_path, OS_PATH_SEPARATOR, - adb_basename(src_path).c_str()); + android::base::Basename(src_path).c_str()); dst_path = path_holder.c_str(); } diff --git a/adb/file_sync_service.cpp b/adb/file_sync_service.cpp index e667bf883..2acf661b7 100644 --- a/adb/file_sync_service.cpp +++ b/adb/file_sync_service.cpp @@ -31,6 +31,7 @@ #include #include +#include #include #include #include @@ -206,7 +207,7 @@ static bool handle_send_file(int s, const char* path, uid_t uid, gid_t gid, uint int fd = adb_open_mode(path, O_WRONLY | O_CREAT | O_EXCL | O_CLOEXEC, mode); if (fd < 0 && errno == ENOENT) { - if (!secure_mkdirs(adb_dirname(path))) { + if (!secure_mkdirs(android::base::Dirname(path))) { SendSyncFailErrno(s, "secure_mkdirs failed"); goto fail; } @@ -333,7 +334,7 @@ static bool handle_send_link(int s, const std::string& path, std::vector& ret = symlink(&buffer[0], path.c_str()); if (ret && errno == ENOENT) { - if (!secure_mkdirs(adb_dirname(path))) { + if (!secure_mkdirs(android::base::Dirname(path))) { SendSyncFailErrno(s, "secure_mkdirs failed"); return false; } diff --git a/base/file.cpp b/base/file.cpp index 6284b04c9..32c243957 100644 --- a/base/file.cpp +++ b/base/file.cpp @@ -18,11 +18,13 @@ #include #include +#include #include #include #include #include +#include #include #include @@ -236,5 +238,56 @@ std::string GetExecutablePath() { #endif } +std::string Basename(const std::string& path) { + + // Copy path because basename may modify the string passed in. + std::string result(path); + +#if !defined(__BIONIC__) + // Use lock because basename() may write to a process global and return a + // pointer to that. Note that this locking strategy only works if all other + // callers to basename in the process also grab this same lock, but its + // better than nothing. Bionic's basename returns a thread-local buffer. + static std::mutex& basename_lock = *new std::mutex(); + std::lock_guard lock(basename_lock); +#endif + + // Note that if std::string uses copy-on-write strings, &str[0] will cause + // the copy to be made, so there is no chance of us accidentally writing to + // the storage for 'path'. + char* name = basename(&result[0]); + + // In case basename returned a pointer to a process global, copy that string + // before leaving the lock. + result.assign(name); + + return result; +} + +std::string Dirname(const std::string& path) { + // Copy path because dirname may modify the string passed in. + std::string result(path); + +#if !defined(__BIONIC__) + // Use lock because dirname() may write to a process global and return a + // pointer to that. Note that this locking strategy only works if all other + // callers to dirname in the process also grab this same lock, but its + // better than nothing. Bionic's dirname returns a thread-local buffer. + static std::mutex& dirname_lock = *new std::mutex(); + std::lock_guard lock(dirname_lock); +#endif + + // Note that if std::string uses copy-on-write strings, &str[0] will cause + // the copy to be made, so there is no chance of us accidentally writing to + // the storage for 'path'. + char* parent = dirname(&result[0]); + + // In case dirname returned a pointer to a process global, copy that string + // before leaving the lock. + result.assign(parent); + + return result; +} + } // namespace base } // namespace android diff --git a/base/file_test.cpp b/base/file_test.cpp index ed39ce973..bbd503783 100644 --- a/base/file_test.cpp +++ b/base/file_test.cpp @@ -162,3 +162,15 @@ TEST(file, Readlink) { TEST(file, GetExecutablePath) { ASSERT_NE("", android::base::GetExecutablePath()); } + +TEST(file, Basename) { + EXPECT_EQ("sh", android::base::Basename("/system/bin/sh")); + EXPECT_EQ("sh", android::base::Basename("sh")); + EXPECT_EQ("sh", android::base::Basename("/system/bin/sh/")); +} + +TEST(file, Dirname) { + EXPECT_EQ("/system/bin", android::base::Dirname("/system/bin/sh")); + EXPECT_EQ(".", android::base::Dirname("sh")); + EXPECT_EQ("/system/bin", android::base::Dirname("/system/bin/sh/")); +} diff --git a/base/include/android-base/file.h b/base/include/android-base/file.h index cd64262e1..c7e094e13 100644 --- a/base/include/android-base/file.h +++ b/base/include/android-base/file.h @@ -52,6 +52,11 @@ bool Readlink(const std::string& path, std::string* result); std::string GetExecutablePath(); +// Like the regular basename and dirname, but thread-safe on all +// platforms and capable of correctly handling exotic Windows paths. +std::string Basename(const std::string& path); +std::string Dirname(const std::string& path); + } // namespace base } // namespace android