[Lldb-commits] [PATCH] D152855: [lldb][Android] Add PlatformAndroidTest

[Greg Clayton via Phabricator via lldb-commits]

clayborg added inline comments.


================
Comment at: lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp:204
   // constraints - try "cat ..." as a fallback.
-  AdbClient adb(m_device_id);
+  AdbClientUP adb(GetAdbClient(error));
+  if (error.Fail())
----------------
Do we want the PlatformAndroid object to have a member variable that stores the AdbClientUP as a member variable so we don't need to recreate this all the time? If the object isn't expensive to create and destroy, no worries, but if it is, then we might want to have a member variable


================
Comment at: lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp:404
 
+PlatformAndroid::AdbClientUP PlatformAndroid::GetAdbClient(Status &error) {
+  AdbClientUP adb(std::make_unique<AdbClient>(m_device_id));
----------------
We could cache the AdbClient object and return just a pointer here. The idea would be to have a member variable in PlatformAndroid object and then return just a "AdbClient *" from this function. This would stop us from creating and destroying a AdbClient object each time this is called.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D152855/new/

https://reviews.llvm.org/D152855