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

[Alex Langford via Phabricator via lldb-commits]

bulbazord added inline comments.


================
Comment at: lldb/source/Plugins/Platform/Android/PlatformAndroid.cpp:209
 
-  return adb.ShellToFile(cmd, minutes(1), destination);
+  return adb->ShellToFile(cmd, minutes(1), destination);
 }
----------------
`GetAdbClient()` calls `make_unique` which can fail. You'll want to add some checks when using all the `adb` objects in this file.


================
Comment at: lldb/source/Plugins/Platform/Android/PlatformAndroid.h:73
 
+  typedef std::unique_ptr<AdbClient> AdbClientSP;
+  virtual AdbClientSP GetAdbClient();
----------------
`SP` -> `shared_ptr`. Was this supposed to be `std::shared_ptr`? If it's supposed to be a unique_ptr, you probably want `AdbClientUP`.


================
Comment at: lldb/source/Plugins/Platform/Android/PlatformAndroid.h:74
+  typedef std::unique_ptr<AdbClient> AdbClientSP;
+  virtual AdbClientSP GetAdbClient();
+
----------------
I feel a little strange about this one because it's possible to create 2 `AdbClient`s from the same PlatformAndroid ID, both are unique pointers and essentially do the same thing. Maybe `PlatformAndroid` can own an `AdbClient` that it vends through `GetAdbClient`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152855