[Lldb-commits] [PATCH] Use Android device serial number instead of hostname as a target identifier within module cache.

[Oleksiy Vyalov]

Please see my comments.


================
Comment at: source/Plugins/Platform/Android/AdbClient.h:33-37
@@ -31,4 +32,7 @@
 
+    static Error
+    CreateByDeviceID (const char* device_id, AdbClient &adb);
+
     AdbClient () = default;
     explicit AdbClient (const std::string &device_id);
 
----------------
tberghammer wrote:
> What is your opinion about using a proper factory pattern instead of this combined approach? If we worry about the performance implication caused by querying the device list every time then we can make that optional.
> 
> Note: I plan to add a few more factory method to create an AdbClient based on -e (emulator) and -d (device) flags the same way as adb do it.
Do you mean to use separate factory methods to attach either to emulator or device?

Performance shouldn't be an issue here since it's one-off call and then device_id is stored inside PlatformAndroid.

Could you give more context why we need to distinguish emulator and device flows? I mean, once we have device_id (either emulator and real device) we can distinguish Android instances even when there are a few connected devices. As for me, from LLDB perspective either emulator or device should be treated  identically.


================
Comment at: source/Plugins/Platform/Android/AdbClient.h:39-40
@@ -34,4 +38,4 @@
 
     void
     SetDeviceID (const std::string& device_id);
 
----------------
tberghammer wrote:
> Can we remove this function (or make it private)?
Made it private.

http://reviews.llvm.org/D8597

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/