[Lldb-commits] [PATCH] D49739: [WIP] Re-implement MI target-select command.

[Greg Clayton via Phabricator via lldb-commits]

clayborg added inline comments.


================
Comment at: include/lldb/API/SBTarget.h:275-276
 
+  void AppendImageSearchPath(const char *from, const char *to);
+
+  void AppendImageSearchPath(const char *from, const char *to,
----------------
Remove this first one? Other functions were first created and gave no error feedback and then another version was added that had an error. We should start with one that has an error and not ever add this version.


================
Comment at: include/lldb/Target/Target.h:1003-1005
+  void AppendImageSearchPath(const ConstString &from,
+                             const ConstString &to);
+
----------------
Remove and just use "PathMappingList &Target::GetImageSearchPathList();"



================
Comment at: lit/tools/lldb-mi/target/inputs/target-select-so-path.sh:3-11
+function get_random_unused_port {
+    local port=$(shuf -i 2000-65000 -n 1)
+    netstat -lat | grep $port > /dev/null
+    if [[ $? == 1 ]] ; then
+        echo $port
+    else
+        get_random_unused_port
----------------
This seems like it could be racy and cause problems on buildbots.


================
Comment at: source/API/SBTarget.cpp:1460-1464
+void SBTarget::AppendImageSearchPath(const char *from, const char *to) {
+  lldb::SBError error; // Ignored
+  AppendImageSearchPath(from, to, error);
+}
+
----------------
Remove this version that takes no error


================
Comment at: source/Target/Target.cpp:2055-2059
+void Target::AppendImageSearchPath(const ConstString &from,
+                                   const ConstString &to) {
+  m_image_search_paths.Append(from, to, true);
+}
+
----------------
You don't need this, just call "target. GetImageSearchPathList().Append(...)


https://reviews.llvm.org/D49739