Skip to content

Commit ba8be29

Browse files
committed
address comments
1 parent 0f906e2 commit ba8be29

File tree

3 files changed

+47
-52
lines changed

3 files changed

+47
-52
lines changed

lldb/source/Plugins/Platform/Android/AdbClient.cpp

Lines changed: 38 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,8 @@
88

99
#include "AdbClient.h"
1010

11+
#include "lldb/Host/ConnectionFileDescriptor.h"
1112
#include "lldb/Host/FileSystem.h"
12-
#include "lldb/Host/posix/ConnectionFileDescriptorPosix.h"
1313
#include "lldb/Utility/Connection.h"
1414
#include "lldb/Utility/DataEncoder.h"
1515
#include "lldb/Utility/DataExtractor.h"
@@ -35,19 +35,19 @@ using namespace lldb_private::platform_android;
3535
using namespace std::chrono;
3636
using namespace llvm;
3737

38-
const static char *kSocketNamespaceAbstract = "localabstract";
39-
const static char *kSocketNamespaceFileSystem = "localfilesystem";
38+
static const char *kSocketNamespaceAbstract = "localabstract";
39+
static const char *kSocketNamespaceFileSystem = "localfilesystem";
4040
const seconds kReadTimeout(20);
41-
const static char *kOKAY = "OKAY";
42-
const static char *kFAIL = "FAIL";
43-
const static char *kDATA = "DATA";
44-
const static char *kDONE = "DONE";
45-
const static char *kSEND = "SEND";
46-
const static char *kRECV = "RECV";
47-
const static char *kSTAT = "STAT";
48-
const static size_t kSyncPacketLen = 8;
49-
const static size_t kMaxPushData = 2 * 1024;
50-
const static uint32_t kDefaultMode = 0100770;
41+
static const char *kOKAY = "OKAY";
42+
static const char *kFAIL = "FAIL";
43+
static const char *kDATA = "DATA";
44+
static const char *kDONE = "DONE";
45+
static const char *kSEND = "SEND";
46+
static const char *kRECV = "RECV";
47+
static const char *kSTAT = "STAT";
48+
static const size_t kSyncPacketLen = 8;
49+
static const size_t kMaxPushData = 2 * 1024;
50+
static const uint32_t kDefaultMode = 0100770;
5151

5252
static Status ReadAllBytes(Connection &conn, void *buffer, size_t size) {
5353
Status error;
@@ -129,7 +129,7 @@ static Status ReadResponseStatus(Connection &conn) {
129129
return error;
130130
}
131131

132-
static Status SendAdbMessage(Connection &conn, const std::string &packet) {
132+
static Status SendAdbMessage(Connection &conn, llvm::StringRef packet) {
133133
Status error;
134134

135135
char length_buffer[5];
@@ -142,7 +142,7 @@ static Status SendAdbMessage(Connection &conn, const std::string &packet) {
142142
if (error.Fail())
143143
return error;
144144

145-
conn.Write(packet.c_str(), packet.size(), status, &error);
145+
conn.Write(packet.str().c_str(), packet.size(), status, &error);
146146
return error;
147147
}
148148

@@ -168,13 +168,12 @@ static Status EnterSyncMode(Connection &conn) {
168168
return ReadResponseStatus(conn);
169169
}
170170

171-
static Status SelectTargetDevice(Connection &conn,
172-
const std::string &device_id) {
171+
static Status SelectTargetDevice(Connection &conn, llvm::StringRef device_id) {
173172
Log *log = GetLog(LLDBLog::Platform);
174-
LLDB_LOGF(log, "Selecting device: %s", device_id.c_str());
173+
LLDB_LOGF(log, "Selecting device: %s", device_id.str().c_str());
175174

176175
std::ostringstream msg;
177-
msg << "host:transport:" << device_id;
176+
msg << "host:transport:" << device_id.str();
178177

179178
auto error = SendAdbMessage(conn, msg.str());
180179
if (error.Fail())
@@ -185,11 +184,10 @@ static Status SelectTargetDevice(Connection &conn,
185184

186185
Expected<std::string> AdbClient::ResolveDeviceID(StringRef device_id) {
187186
StringRef preferred_serial;
188-
if (!device_id.empty()) {
187+
if (!device_id.empty())
189188
preferred_serial = device_id;
190-
} else if (const char *env_serial = std::getenv("ANDROID_SERIAL")) {
189+
else if (const char *env_serial = std::getenv("ANDROID_SERIAL"))
191190
preferred_serial = env_serial;
192-
}
193191

194192
if (preferred_serial.empty()) {
195193
DeviceIDList connected_devices;
@@ -250,12 +248,12 @@ Expected<std::string> AdbClient::ResolveDeviceID(StringRef device_id) {
250248
return resolved_device_id;
251249
}
252250

253-
AdbClient::AdbClient(const std::string &device_id) : m_device_id(device_id) {
251+
AdbClient::AdbClient(llvm::StringRef device_id) : m_device_id(device_id) {
254252
Log *log = GetLog(LLDBLog::Platform);
255253
LLDB_LOGF(log,
256254
"AdbClient::AdbClient(device_id='%s') - Creating AdbClient with "
257255
"device ID",
258-
device_id.c_str());
256+
device_id.str().c_str());
259257
m_conn = std::make_unique<ConnectionFileDescriptor>();
260258
Connect();
261259
}
@@ -276,7 +274,7 @@ AdbClient::~AdbClient() {
276274
m_device_id.c_str());
277275
}
278276

279-
const std::string &AdbClient::GetDeviceID() const { return m_device_id; }
277+
llvm::StringRef AdbClient::GetDeviceID() const { return m_device_id; }
280278

281279
Status AdbClient::Connect() {
282280
if (m_conn->IsConnected())
@@ -328,9 +326,9 @@ Status AdbClient::DeletePortForwarding(const uint16_t local_port) {
328326
return ReadResponseStatus(*m_conn);
329327
}
330328

331-
Status AdbClient::SendDeviceMessage(const std::string &packet) {
329+
Status AdbClient::SendDeviceMessage(llvm::StringRef packet) {
332330
std::ostringstream msg;
333-
msg << "host-serial:" << m_device_id << ":" << packet;
331+
msg << "host-serial:" << m_device_id << ":" << packet.str();
334332
return SendAdbMessage(*m_conn, msg.str());
335333
}
336334

@@ -430,8 +428,8 @@ Status AdbClient::ShellToFile(const char *command, milliseconds timeout,
430428
return Status();
431429
}
432430

433-
Status AdbSyncService::internalPullFile(const FileSpec &remote_file,
434-
const FileSpec &local_file) {
431+
Status AdbSyncService::PullFileImpl(const FileSpec &remote_file,
432+
const FileSpec &local_file) {
435433
const auto local_file_path = local_file.GetPath();
436434
llvm::FileRemover local_file_remover(local_file_path);
437435

@@ -465,8 +463,8 @@ Status AdbSyncService::internalPullFile(const FileSpec &remote_file,
465463
return error;
466464
}
467465

468-
Status AdbSyncService::internalPushFile(const FileSpec &local_file,
469-
const FileSpec &remote_file) {
466+
Status AdbSyncService::PushFileImpl(const FileSpec &local_file,
467+
const FileSpec &remote_file) {
470468
const auto local_file_path(local_file.GetPath());
471469
std::ifstream src(local_file_path.c_str(), std::ios::in | std::ios::binary);
472470
if (!src.is_open())
@@ -523,8 +521,8 @@ Status AdbSyncService::internalPushFile(const FileSpec &local_file,
523521
return error;
524522
}
525523

526-
Status AdbSyncService::internalStat(const FileSpec &remote_file, uint32_t &mode,
527-
uint32_t &size, uint32_t &mtime) {
524+
Status AdbSyncService::StatImpl(const FileSpec &remote_file, uint32_t &mode,
525+
uint32_t &size, uint32_t &mtime) {
528526
const std::string remote_file_path(remote_file.GetPath(false));
529527
auto error = SendSyncRequest(kSTAT, remote_file_path.length(),
530528
remote_file_path.c_str());
@@ -561,22 +559,22 @@ Status AdbSyncService::internalStat(const FileSpec &remote_file, uint32_t &mode,
561559

562560
Status AdbSyncService::PullFile(const FileSpec &remote_file,
563561
const FileSpec &local_file) {
564-
return executeCommand([this, &remote_file, &local_file]() {
565-
return internalPullFile(remote_file, local_file);
562+
return ExecuteCommand([this, &remote_file, &local_file]() {
563+
return PullFileImpl(remote_file, local_file);
566564
});
567565
}
568566

569567
Status AdbSyncService::PushFile(const FileSpec &local_file,
570568
const FileSpec &remote_file) {
571-
return executeCommand([this, &local_file, &remote_file]() {
572-
return internalPushFile(local_file, remote_file);
569+
return ExecuteCommand([this, &local_file, &remote_file]() {
570+
return PushFileImpl(local_file, remote_file);
573571
});
574572
}
575573

576574
Status AdbSyncService::Stat(const FileSpec &remote_file, uint32_t &mode,
577575
uint32_t &size, uint32_t &mtime) {
578-
return executeCommand([this, &remote_file, &mode, &size, &mtime]() {
579-
return internalStat(remote_file, mode, size, mtime);
576+
return ExecuteCommand([this, &remote_file, &mode, &size, &mtime]() {
577+
return StatImpl(remote_file, mode, size, mtime);
580578
});
581579
}
582580

@@ -594,7 +592,7 @@ AdbSyncService::AdbSyncService(const std::string device_id)
594592
m_device_id.c_str());
595593
}
596594

597-
Status AdbSyncService::executeCommand(const std::function<Status()> &cmd) {
595+
Status AdbSyncService::ExecuteCommand(const std::function<Status()> &cmd) {
598596
Status error = cmd();
599597
return error;
600598
}

lldb/source/Plugins/Platform/Android/AdbClient.h

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -43,11 +43,11 @@ class AdbClient {
4343
static llvm::Expected<std::string> ResolveDeviceID(llvm::StringRef device_id);
4444

4545
AdbClient();
46-
explicit AdbClient(const std::string &device_id);
46+
explicit AdbClient(llvm::StringRef device_id);
4747

4848
virtual ~AdbClient();
4949

50-
const std::string &GetDeviceID() const;
50+
llvm::StringRef GetDeviceID() const;
5151

5252
Status SetPortForwarding(const uint16_t local_port,
5353
const uint16_t remote_port);
@@ -68,7 +68,7 @@ class AdbClient {
6868
Status Connect();
6969

7070
private:
71-
Status SendDeviceMessage(const std::string &packet);
71+
Status SendDeviceMessage(llvm::StringRef packet);
7272

7373
Status ReadMessageStream(std::vector<char> &message,
7474
std::chrono::milliseconds timeout);
@@ -94,20 +94,18 @@ class AdbSyncService {
9494
uint32_t &size, uint32_t &mtime);
9595
virtual bool IsConnected() const;
9696

97-
const std::string &GetDeviceId() const { return m_device_id; }
97+
llvm::StringRef GetDeviceId() const { return m_device_id; }
9898

9999
private:
100100
Status SendSyncRequest(const char *request_id, const uint32_t data_len,
101101
const void *data);
102102
Status ReadSyncHeader(std::string &response_id, uint32_t &data_len);
103103
Status PullFileChunk(std::vector<char> &buffer, bool &eof);
104-
Status internalPullFile(const FileSpec &remote_file,
105-
const FileSpec &local_file);
106-
Status internalPushFile(const FileSpec &local_file,
107-
const FileSpec &remote_file);
108-
Status internalStat(const FileSpec &remote_file, uint32_t &mode,
109-
uint32_t &size, uint32_t &mtime);
110-
Status executeCommand(const std::function<Status()> &cmd);
104+
Status PullFileImpl(const FileSpec &remote_file, const FileSpec &local_file);
105+
Status PushFileImpl(const FileSpec &local_file, const FileSpec &remote_file);
106+
Status StatImpl(const FileSpec &remote_file, uint32_t &mode, uint32_t &size,
107+
uint32_t &mtime);
108+
Status ExecuteCommand(const std::function<Status()> &cmd);
111109

112110
std::unique_ptr<Connection> m_conn;
113111
std::string m_device_id;

lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@ using namespace lldb;
2424
using namespace lldb_private;
2525
using namespace lldb_private::platform_android;
2626
using namespace std::chrono;
27-
using namespace llvm;
2827

2928
LLDB_PLUGIN_DEFINE(PlatformAndroid)
3029

0 commit comments

Comments
 (0)