From 787f3442cc7d7b1f07189d77413893e2c2c81447 Mon Sep 17 00:00:00 2001 From: Josh Gao Date: Tue, 3 Nov 2015 18:52:35 -0800 Subject: [PATCH] adb: make adb_basename match the POSIX behavior. Previously, adb_basename was behaving according to the GNU, POSIX-incompatible basename, despite POSIX adb_dirname existing alongside it. This patch changes adb_basename to pass through to the POSIX basename. Bug: http://b/25456821 Change-Id: I62a4865cccf3b9cdbc112e3e53ff475aa4a23bd9 --- adb/adb_utils.cpp | 29 ++++++++++++++++++++++++----- adb/adb_utils_test.cpp | 7 +++++++ 2 files changed, 31 insertions(+), 5 deletions(-) diff --git a/adb/adb_utils.cpp b/adb/adb_utils.cpp index f7b2e4e29..6817234b3 100644 --- a/adb/adb_utils.cpp +++ b/adb/adb_utils.cpp @@ -33,6 +33,7 @@ #include "adb_trace.h" #include "sysdeps.h" +ADB_MUTEX_DEFINE(basename_lock); ADB_MUTEX_DEFINE(dirname_lock); bool getcwd(std::string* s) { @@ -69,13 +70,31 @@ std::string escape_arg(const std::string& s) { } std::string adb_basename(const std::string& path) { - size_t base = path.find_last_of(OS_PATH_SEPARATORS); - return (base != std::string::npos) ? path.substr(base + 1) : path; + // 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 dirname in the process also grab this same lock. + adb_mutex_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); + + adb_mutex_unlock(&basename_lock); + + return result; } std::string adb_dirname(const std::string& path) { // Copy path because dirname may modify the string passed in. - std::string parent_storage(path); + 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 @@ -85,11 +104,11 @@ std::string adb_dirname(const std::string& path) { // 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(&parent_storage[0]); + char* parent = dirname(&result[0]); // In case dirname returned a pointer to a process global, copy that string // before leaving the lock. - const std::string result(parent); + result.assign(parent); adb_mutex_unlock(&dirname_lock); diff --git a/adb/adb_utils_test.cpp b/adb/adb_utils_test.cpp index 4a2787a52..93c20cb00 100644 --- a/adb/adb_utils_test.cpp +++ b/adb/adb_utils_test.cpp @@ -102,6 +102,13 @@ TEST(adb_utils, escape_arg) { 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/")); } TEST(adb_utils, parse_host_and_port) {