[Lldb-commits] [lldb] [lldb] refactor PlatformAndroid (PR #145382)

[Pavel Labath via lldb-commits]

================
@@ -108,62 +55,73 @@ Status AdbClient::CreateByDeviceID(const std::string &device_id,
           "Expected a single connected device, got instead %zu - try "
           "setting 'ANDROID_SERIAL'",
           connected_devices.size());
-    adb.SetDeviceID(connected_devices.front());
+    
+    resolved_device_id = std::move(connected_devices.front());
   } else {
-    adb.SetDeviceID(android_serial);
+    resolved_device_id = preferred_serial.str();
   }
+  
+  Log *log = GetLog(LLDBLog::Platform);
+  LLDB_LOGF(log, "AdbClient::ResolveDeviceID Resolved device ID: %s", 
+            resolved_device_id.c_str());
   return error;
 }
 
-AdbClient::AdbClient() = default;
 
-AdbClient::AdbClient(const std::string &device_id) : m_device_id(device_id) {}
+AdbClient::AdbClient(const std::string &device_id) : m_device_id(device_id) {
+  Log *log = GetLog(LLDBLog::Platform);
+  LLDB_LOGF(log, "AdbClient::AdbClient(device_id='%s') - Creating AdbClient with device ID", 
+              device_id.c_str());
+  m_conn = std::make_unique<ConnectionFileDescriptor>();
+  Connect();
+}
 
-AdbClient::~AdbClient() = default;
+AdbClient::AdbClient() {
+  Log *log = GetLog(LLDBLog::Platform);
+  LLDB_LOGF(log, "AdbClient::AdbClient() - Creating AdbClient with default constructor");
+  m_conn = std::make_unique<ConnectionFileDescriptor>();
+  Connect();
+}
 
-void AdbClient::SetDeviceID(const std::string &device_id) {
-  m_device_id = device_id;
+AdbClient::~AdbClient() {
+  Log *log = GetLog(LLDBLog::Platform);
+  LLDB_LOGF(log, "AdbClient::~AdbClient() - Destroying AdbClient for device: %s", 
+              m_device_id.c_str());
 }
 
 const std::string &AdbClient::GetDeviceID() const { return m_device_id; }
 
 Status AdbClient::Connect() {
-  Status error;
-  m_conn = std::make_unique<ConnectionFileDescriptor>();
-  std::string port = "5037";
-  if (const char *env_port = std::getenv("ANDROID_ADB_SERVER_PORT")) {
-    port = env_port;
-  }
-  std::string uri = "connect://127.0.0.1:" + port;
-  m_conn->Connect(uri.c_str(), &error);
+  if (m_conn->IsConnected())
+    return Status();
 
-  return error;
+  return ConnectToAdb(*m_conn);
 }
 
 Status AdbClient::GetDevices(DeviceIDList &device_list) {
   device_list.clear();
 
-  auto error = SendMessage("host:devices");
+  auto error = SendAdbMessage(*m_conn, "host:devices");
   if (error.Fail())
     return error;
-
-  error = ReadResponseStatus();
+  
+  error = ReadResponseStatus(*m_conn);
   if (error.Fail())
-    return error;
+  return error;
 
-  std::vector<char> in_buffer;
-  error = ReadMessage(in_buffer);
+std::vector<char> in_buffer;
+error = ReadAdbMessage(*m_conn, in_buffer);
 
-  llvm::StringRef response(&in_buffer[0], in_buffer.size());
-  llvm::SmallVector<llvm::StringRef, 4> devices;
-  response.split(devices, "\n", -1, false);
+llvm::StringRef response(&in_buffer[0], in_buffer.size());
+llvm::SmallVector<llvm::StringRef, 4> devices;
+response.split(devices, "\n", -1, false);
 
-  for (const auto &device : devices)
-    device_list.push_back(std::string(device.split('\t').first));
+for (const auto &device : devices)
+device_list.push_back(std::string(device.split('\t').first));
 
-  // Force disconnect since ADB closes connection after host:devices response
-  // is sent.
-  m_conn.reset();
+// WARNING: ADB closes the connection after host:devices response.
+// This AdbClient instance is now INVALID and should not be used for any further operations.
+// This method should ONLY be called from ResolveDeviceID() which uses a temporary AdbClient.
----------------
labath wrote:

Does this need to be a member function. Could it be a static function defined in AdbClient.cpp, or possibly inlined into `ResolveDeviceID` ?

https://github.com/llvm/llvm-project/pull/145382