adb: Remove most C-style allocations
This change gets rid of most malloc/calloc/free calls. The future is
now!
Bug: None
Test: test_device.py
Change-Id: Iccfe3bd4fe45a0319bd9f23b8cbff4c7070c9f4d
diff --git a/adb/adb.cpp b/adb/adb.cpp
index 20d0db5..9b48702 100644
--- a/adb/adb.cpp
+++ b/adb/adb.cpp
@@ -264,15 +264,6 @@
send_packet(cp, t);
}
-// qual_overwrite is used to overwrite a qualifier string. dst is a
-// pointer to a char pointer. It is assumed that if *dst is non-NULL, it
-// was malloc'ed and needs to freed. *dst will be set to a dup of src.
-// TODO: switch to std::string for these atransport fields instead.
-static void qual_overwrite(char** dst, const std::string& src) {
- free(*dst);
- *dst = strdup(src.c_str());
-}
-
void parse_banner(const std::string& banner, atransport* t) {
D("parse_banner: %s", banner.c_str());
@@ -296,11 +287,11 @@
const std::string& key = key_value[0];
const std::string& value = key_value[1];
if (key == "ro.product.name") {
- qual_overwrite(&t->product, value);
+ t->product = value;
} else if (key == "ro.product.model") {
- qual_overwrite(&t->model, value);
+ t->model = value;
} else if (key == "ro.product.device") {
- qual_overwrite(&t->device, value);
+ t->device = value;
} else if (key == "features") {
t->SetFeatures(value);
}
@@ -424,8 +415,8 @@
/* Other READY messages must use the same local-id */
s->ready(s);
} else {
- D("Invalid A_OKAY(%d,%d), expected A_OKAY(%d,%d) on transport %s",
- p->msg.arg0, p->msg.arg1, s->peer->id, p->msg.arg1, t->serial);
+ D("Invalid A_OKAY(%d,%d), expected A_OKAY(%d,%d) on transport %s", p->msg.arg0,
+ p->msg.arg1, s->peer->id, p->msg.arg1, t->serial.c_str());
}
} else {
// When receiving A_OKAY from device for A_OPEN request, the host server may
@@ -451,8 +442,8 @@
* socket has a peer on the same transport.
*/
if (p->msg.arg0 == 0 && s->peer && s->peer->transport != t) {
- D("Invalid A_CLSE(0, %u) from transport %s, expected transport %s",
- p->msg.arg1, t->serial, s->peer->transport->serial);
+ D("Invalid A_CLSE(0, %u) from transport %s, expected transport %s", p->msg.arg1,
+ t->serial.c_str(), s->peer->transport->serial.c_str());
} else {
s->close(s);
}
@@ -1171,7 +1162,7 @@
std::string error;
atransport* t = acquire_one_transport(type, serial, transport_id, nullptr, &error);
if (t) {
- return SendOkay(reply_fd, t->serial ? t->serial : "unknown");
+ return SendOkay(reply_fd, !t->serial.empty() ? t->serial : "unknown");
} else {
return SendFail(reply_fd, error);
}
@@ -1180,7 +1171,7 @@
std::string error;
atransport* t = acquire_one_transport(type, serial, transport_id, nullptr, &error);
if (t) {
- return SendOkay(reply_fd, t->devpath ? t->devpath : "unknown");
+ return SendOkay(reply_fd, !t->devpath.empty() ? t->devpath : "unknown");
} else {
return SendFail(reply_fd, error);
}
diff --git a/adb/adb_listeners.cpp b/adb/adb_listeners.cpp
index ea5a44e..f4a92e3 100644
--- a/adb/adb_listeners.cpp
+++ b/adb/adb_listeners.cpp
@@ -136,9 +136,10 @@
}
// <device-serial> " " <local-name> " " <remote-name> "\n"
// Entries from "adb reverse" have no serial.
- android::base::StringAppendF(&result, "%s %s %s\n",
- l->transport->serial ? l->transport->serial : "(reverse)",
- l->local_name.c_str(), l->connect_to.c_str());
+ android::base::StringAppendF(
+ &result, "%s %s %s\n",
+ !l->transport->serial.empty() ? l->transport->serial.c_str() : "(reverse)",
+ l->local_name.c_str(), l->connect_to.c_str());
}
return result;
}
diff --git a/adb/client/commandline.cpp b/adb/client/commandline.cpp
index 80d0dd3..7791895 100644
--- a/adb/client/commandline.cpp
+++ b/adb/client/commandline.cpp
@@ -362,9 +362,8 @@
}
static void copy_to_file(int inFd, int outFd) {
- const size_t BUFSIZE = 32 * 1024;
- char* buf = (char*) malloc(BUFSIZE);
- if (buf == nullptr) fatal("couldn't allocate buffer for copy_to_file");
+ constexpr size_t BUFSIZE = 32 * 1024;
+ std::vector<char> buf(BUFSIZE);
int len;
long total = 0;
int old_stdin_mode = -1;
@@ -376,9 +375,9 @@
while (true) {
if (inFd == STDIN_FILENO) {
- len = unix_read(inFd, buf, BUFSIZE);
+ len = unix_read(inFd, buf.data(), BUFSIZE);
} else {
- len = adb_read(inFd, buf, BUFSIZE);
+ len = adb_read(inFd, buf.data(), BUFSIZE);
}
if (len == 0) {
D("copy_to_file() : read 0 bytes; exiting");
@@ -389,10 +388,10 @@
break;
}
if (outFd == STDOUT_FILENO) {
- fwrite(buf, 1, len, stdout);
+ fwrite(buf.data(), 1, len, stdout);
fflush(stdout);
} else {
- adb_write(outFd, buf, len);
+ adb_write(outFd, buf.data(), len);
}
total += len;
}
@@ -400,7 +399,6 @@
stdinout_raw_epilogue(inFd, outFd, old_stdin_mode, old_stdout_mode);
D("copy_to_file() finished after %lu bytes", total);
- free(buf);
}
static void send_window_size_change(int fd, std::unique_ptr<ShellProtocol>& shell) {
@@ -1142,24 +1140,22 @@
static void write_zeros(int bytes, int fd) {
int old_stdin_mode = -1;
int old_stdout_mode = -1;
- char* buf = (char*) calloc(1, bytes);
- if (buf == nullptr) fatal("couldn't allocate buffer for write_zeros");
+ std::vector<char> buf(bytes);
D("write_zeros(%d) -> %d", bytes, fd);
stdinout_raw_prologue(-1, fd, old_stdin_mode, old_stdout_mode);
if (fd == STDOUT_FILENO) {
- fwrite(buf, 1, bytes, stdout);
+ fwrite(buf.data(), 1, bytes, stdout);
fflush(stdout);
} else {
- adb_write(fd, buf, bytes);
+ adb_write(fd, buf.data(), bytes);
}
stdinout_raw_prologue(-1, fd, old_stdin_mode, old_stdout_mode);
D("write_zeros() finished");
- free(buf);
}
static int backup(int argc, const char** argv) {
diff --git a/adb/sysdeps/memory.h b/adb/sysdeps/memory.h
index 0e4c509..4108aff 100644
--- a/adb/sysdeps/memory.h
+++ b/adb/sysdeps/memory.h
@@ -23,6 +23,23 @@
// We don't have C++14 on Windows yet.
// Reimplement std::make_unique ourselves until we do.
+namespace internal {
+
+template <typename T>
+struct array_known_bounds;
+
+template <typename T>
+struct array_known_bounds<T[]> {
+ constexpr static bool value = false;
+};
+
+template <typename T, size_t N>
+struct array_known_bounds<T[N]> {
+ constexpr static bool value = true;
+};
+
+} // namespace internal
+
namespace std {
template <typename T, typename... Args>
@@ -31,6 +48,18 @@
return std::unique_ptr<T>(new T(std::forward<Args>(args)...));
}
+template <typename T>
+typename std::enable_if<std::is_array<T>::value && !internal::array_known_bounds<T>::value,
+ std::unique_ptr<T>>::type
+make_unique(std::size_t size) {
+ return std::unique_ptr<T>(new typename std::remove_extent<T>::type[size]());
+}
+
+template <typename T, typename... Args>
+typename std::enable_if<std::is_array<T>::value && internal::array_known_bounds<T>::value,
+ std::unique_ptr<T>>::type
+make_unique(Args&&... args) = delete;
+
} // namespace std
#endif
diff --git a/adb/transport.cpp b/adb/transport.cpp
index f2c40d2..638940c 100644
--- a/adb/transport.cpp
+++ b/adb/transport.cpp
@@ -173,18 +173,18 @@
attempt = reconnect_queue_.front();
reconnect_queue_.pop();
if (attempt.transport->kicked()) {
- D("transport %s was kicked. giving up on it.", attempt.transport->serial);
+ D("transport %s was kicked. giving up on it.", attempt.transport->serial.c_str());
remove_transport(attempt.transport);
continue;
}
}
- D("attempting to reconnect %s", attempt.transport->serial);
+ D("attempting to reconnect %s", attempt.transport->serial.c_str());
if (!attempt.transport->Reconnect()) {
- D("attempting to reconnect %s failed.", attempt.transport->serial);
+ D("attempting to reconnect %s failed.", attempt.transport->serial.c_str());
if (attempt.attempts_left == 0) {
D("transport %s exceeded the number of retry attempts. giving up on it.",
- attempt.transport->serial);
+ attempt.transport->serial.c_str());
remove_transport(attempt.transport);
continue;
}
@@ -197,7 +197,7 @@
continue;
}
- D("reconnection to %s succeeded.", attempt.transport->serial);
+ D("reconnection to %s succeeded.", attempt.transport->serial.c_str());
register_transport(attempt.transport);
}
}
@@ -402,14 +402,14 @@
p->msg.data_check = calculate_apacket_checksum(p);
}
- VLOG(TRANSPORT) << dump_packet(t->serial, "to remote", p);
+ VLOG(TRANSPORT) << dump_packet(t->serial.c_str(), "to remote", p);
if (t == nullptr) {
fatal("Transport is null");
}
if (t->Write(p) != 0) {
- D("%s: failed to enqueue packet, closing transport", t->serial);
+ D("%s: failed to enqueue packet, closing transport", t->serial.c_str());
t->Kick();
}
}
@@ -619,19 +619,13 @@
t = m.transport;
if (m.action == 0) {
- D("transport: %s deleting", t->serial);
+ D("transport: %s deleting", t->serial.c_str());
{
std::lock_guard<std::recursive_mutex> lock(transport_lock);
transport_list.remove(t);
}
- if (t->product) free(t->product);
- if (t->serial) free(t->serial);
- if (t->model) free(t->model);
- if (t->device) free(t->device);
- if (t->devpath) free(t->devpath);
-
delete t;
update_transports();
@@ -646,11 +640,11 @@
t->connection()->SetTransportName(t->serial_name());
t->connection()->SetReadCallback([t](Connection*, std::unique_ptr<apacket> p) {
if (!check_header(p.get(), t)) {
- D("%s: remote read: bad header", t->serial);
+ D("%s: remote read: bad header", t->serial.c_str());
return false;
}
- VLOG(TRANSPORT) << dump_packet(t->serial, "from remote", p.get());
+ VLOG(TRANSPORT) << dump_packet(t->serial.c_str(), "from remote", p.get());
apacket* packet = p.release();
// TODO: Does this need to run on the main thread?
@@ -658,7 +652,7 @@
return true;
});
t->connection()->SetErrorCallback([t](Connection*, const std::string& error) {
- D("%s: connection terminated: %s", t->serial, error.c_str());
+ D("%s: connection terminated: %s", t->serial.c_str(), error.c_str());
fdevent_run_on_main_thread([t]() {
handle_offline(t);
transport_unref(t);
@@ -717,7 +711,7 @@
tmsg m;
m.transport = transport;
m.action = 1;
- D("transport: %s registered", transport->serial);
+ D("transport: %s registered", transport->serial.c_str());
if (transport_write_action(transport_registration_send, &m)) {
fatal_errno("cannot write transport registration socket\n");
}
@@ -727,7 +721,7 @@
tmsg m;
m.transport = transport;
m.action = 0;
- D("transport: %s removed", transport->serial);
+ D("transport: %s removed", transport->serial.c_str());
if (transport_write_action(transport_registration_send, &m)) {
fatal_errno("cannot write transport registration socket\n");
}
@@ -743,38 +737,38 @@
if (t->ref_count == 0) {
t->connection()->Stop();
if (t->IsTcpDevice() && !t->kicked()) {
- D("transport: %s unref (attempting reconnection) %d", t->serial, t->kicked());
+ D("transport: %s unref (attempting reconnection) %d", t->serial.c_str(), t->kicked());
reconnect_handler.TrackTransport(t);
} else {
- D("transport: %s unref (kicking and closing)", t->serial);
+ D("transport: %s unref (kicking and closing)", t->serial.c_str());
remove_transport(t);
}
} else {
- D("transport: %s unref (count=%zu)", t->serial, t->ref_count);
+ D("transport: %s unref (count=%zu)", t->serial.c_str(), t->ref_count);
}
}
-static int qual_match(const char* to_test, const char* prefix, const char* qual,
+static int qual_match(const std::string& to_test, const char* prefix, const std::string& qual,
bool sanitize_qual) {
- if (!to_test || !*to_test) /* Return true if both the qual and to_test are null strings. */
- return !qual || !*qual;
+ if (to_test.empty()) /* Return true if both the qual and to_test are empty strings. */
+ return qual.empty();
- if (!qual) return 0;
+ if (qual.empty()) return 0;
+ const char* ptr = to_test.c_str();
if (prefix) {
while (*prefix) {
- if (*prefix++ != *to_test++) return 0;
+ if (*prefix++ != *ptr++) return 0;
}
}
- while (*qual) {
- char ch = *qual++;
+ for (char ch : qual) {
if (sanitize_qual && !isalnum(ch)) ch = '_';
- if (ch != *to_test++) return 0;
+ if (ch != *ptr++) return 0;
}
- /* Everything matched so far. Return true if *to_test is a NUL. */
- return !*to_test;
+ /* Everything matched so far. Return true if *ptr is a NUL. */
+ return !*ptr;
}
atransport* acquire_one_transport(TransportType type, const char* serial, TransportId transport_id,
@@ -921,7 +915,7 @@
void atransport::Kick() {
if (!kicked_.exchange(true)) {
- D("kicking transport %p %s", this, this->serial);
+ D("kicking transport %p %s", this, this->serial.c_str());
this->connection()->Stop();
}
}
@@ -1040,7 +1034,7 @@
}
bool atransport::MatchesTarget(const std::string& target) const {
- if (serial) {
+ if (!serial.empty()) {
if (target == serial) {
return true;
} else if (type == kTransportLocal) {
@@ -1069,10 +1063,9 @@
}
}
- return (devpath && target == devpath) ||
- qual_match(target.c_str(), "product:", product, false) ||
- qual_match(target.c_str(), "model:", model, true) ||
- qual_match(target.c_str(), "device:", device, false);
+ return (target == devpath) || qual_match(target, "product:", product, false) ||
+ qual_match(target, "model:", model, true) ||
+ qual_match(target, "device:", device, false);
}
void atransport::SetConnectionEstablished(bool success) {
@@ -1093,9 +1086,9 @@
return str;
}
-static void append_transport_info(std::string* result, const char* key, const char* value,
+static void append_transport_info(std::string* result, const char* key, const std::string& value,
bool alphanumeric) {
- if (value == nullptr || *value == '\0') {
+ if (value.empty()) {
return;
}
@@ -1105,8 +1098,8 @@
}
static void append_transport(const atransport* t, std::string* result, bool long_listing) {
- const char* serial = t->serial;
- if (!serial || !serial[0]) {
+ std::string serial = t->serial;
+ if (serial.empty()) {
serial = "(no serial number)";
}
@@ -1115,7 +1108,8 @@
*result += '\t';
*result += t->connection_state_name();
} else {
- android::base::StringAppendF(result, "%-22s %s", serial, t->connection_state_name().c_str());
+ android::base::StringAppendF(result, "%-22s %s", serial.c_str(),
+ t->connection_state_name().c_str());
append_transport_info(result, "", t->devpath, false);
append_transport_info(result, "product:", t->product, false);
@@ -1138,7 +1132,7 @@
if (x->type != y->type) {
return x->type < y->type;
}
- return strcmp(x->serial, y->serial) < 0;
+ return x->serial < y->serial;
});
std::string result;
@@ -1181,7 +1175,7 @@
std::unique_lock<std::recursive_mutex> lock(transport_lock);
for (const auto& transport : pending_list) {
- if (transport->serial && strcmp(serial, transport->serial) == 0) {
+ if (strcmp(serial, transport->serial.c_str()) == 0) {
VLOG(TRANSPORT) << "socket transport " << transport->serial
<< " is already in pending_list and fails to register";
delete t;
@@ -1190,7 +1184,7 @@
}
for (const auto& transport : transport_list) {
- if (transport->serial && strcmp(serial, transport->serial) == 0) {
+ if (strcmp(serial, transport->serial.c_str()) == 0) {
VLOG(TRANSPORT) << "socket transport " << transport->serial
<< " is already in transport_list and fails to register";
delete t;
@@ -1199,7 +1193,7 @@
}
pending_list.push_front(t);
- t->serial = strdup(serial);
+ t->serial = serial;
lock.unlock();
@@ -1220,7 +1214,7 @@
std::lock_guard<std::recursive_mutex> lock(transport_lock);
for (auto& t : transport_list) {
- if (t->serial && strcmp(serial, t->serial) == 0) {
+ if (strcmp(serial, t->serial.c_str()) == 0) {
result = t;
break;
}
@@ -1251,11 +1245,11 @@
D("transport: %p init'ing for usb_handle %p (sn='%s')", t, usb, serial ? serial : "");
init_usb_transport(t, usb);
if (serial) {
- t->serial = strdup(serial);
+ t->serial = serial;
}
if (devpath) {
- t->devpath = strdup(devpath);
+ t->devpath = devpath;
}
{
diff --git a/adb/transport.h b/adb/transport.h
index cb20615..e9c9d37 100644
--- a/adb/transport.h
+++ b/adb/transport.h
@@ -238,11 +238,11 @@
TransportType type = kTransportAny;
// Used to identify transports for clients.
- char* serial = nullptr;
- char* product = nullptr;
- char* model = nullptr;
- char* device = nullptr;
- char* devpath = nullptr;
+ std::string serial;
+ std::string product;
+ std::string model;
+ std::string device;
+ std::string devpath;
bool IsTcpDevice() const { return type == kTransportLocal; }
@@ -253,7 +253,7 @@
char token[TOKEN_SIZE] = {};
size_t failed_auth_attempts = 0;
- std::string serial_name() const { return serial ? serial : "<unknown>"; }
+ std::string serial_name() const { return !serial.empty() ? serial : "<unknown>"; }
std::string connection_state_name() const;
void update_version(int version, size_t payload);
diff --git a/adb/transport_test.cpp b/adb/transport_test.cpp
index d987d4f..8c628d8 100644
--- a/adb/transport_test.cpp
+++ b/adb/transport_test.cpp
@@ -86,9 +86,9 @@
ASSERT_EQ(0U, t.features().size());
ASSERT_EQ(kCsHost, t.GetConnectionState());
- ASSERT_EQ(nullptr, t.product);
- ASSERT_EQ(nullptr, t.model);
- ASSERT_EQ(nullptr, t.device);
+ ASSERT_EQ(std::string(), t.product);
+ ASSERT_EQ(std::string(), t.model);
+ ASSERT_EQ(std::string(), t.device);
}
TEST(transport, parse_banner_product_features) {
diff --git a/adb/types.h b/adb/types.h
index c6b3f07..a3e5d48 100644
--- a/adb/types.h
+++ b/adb/types.h
@@ -18,6 +18,7 @@
#include <algorithm>
#include <deque>
+#include <memory>
#include <type_traits>
#include <utility>
#include <vector>
@@ -37,7 +38,7 @@
template <typename Iterator>
Block(Iterator begin, Iterator end) : Block(end - begin) {
- std::copy(begin, end, data_);
+ std::copy(begin, end, data_.get());
}
Block(const Block& copy) = delete;
@@ -73,11 +74,11 @@
void assign(InputIt begin, InputIt end) {
clear();
allocate(end - begin);
- std::copy(begin, end, data_);
+ std::copy(begin, end, data_.get());
}
void clear() {
- free(data_);
+ data_.reset();
capacity_ = 0;
size_ = 0;
}
@@ -86,11 +87,11 @@
size_t size() const { return size_; }
bool empty() const { return size() == 0; }
- char* data() { return data_; }
- const char* data() const { return data_; }
+ char* data() { return data_.get(); }
+ const char* data() const { return data_.get(); }
- char* begin() { return data_; }
- const char* begin() const { return data_; }
+ char* begin() { return data_.get(); }
+ const char* begin() const { return data_.get(); }
char* end() { return data() + size_; }
const char* end() const { return data() + size_; }
@@ -108,13 +109,13 @@
CHECK_EQ(0ULL, capacity_);
CHECK_EQ(0ULL, size_);
if (size != 0) {
- data_ = static_cast<char*>(malloc(size));
+ data_ = std::make_unique<char[]>(size);
capacity_ = size;
size_ = size;
}
}
- char* data_ = nullptr;
+ std::unique_ptr<char[]> data_;
size_t capacity_ = 0;
size_t size_ = 0;
};