[Lldb-commits] [PATCH] D116707: [lldb] fix memory leak in "GetGDBServerRegisterInfoXMLAndProcess"

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Yes, that's the API I had in mind, but check out the inline comments for some problems/improvements.



================
Comment at: lldb/source/Host/common/XML.cpp:141
+      attr_value = (const char *)value;
+      free(value);
+    }
----------------
this should be xmlFree, per the function documentation.


================
Comment at: lldb/source/Host/common/XML.cpp:157-159
+  std::string attr_str = GetAttributeValue(name, "");
+  llvm::StringRef attr(attr_str);
+  return llvm::to_integer(attr, value, base);
----------------
I don't see why we need these temporary variables. std::string is implicitly convertible to a StringRef, and you don't need the value afterwards, so you should be able to pass the GetAttributeValue straight into the to_integer function. Did that not work for some reason?


================
Comment at: lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp:4534
+    std::string main_lm_str = root_element.GetAttributeValue("main-lm");
+    llvm::StringRef main_lm(main_lm_str);
     // FIXME: we're silently ignoring invalid data here
----------------
Same here. `.empty()` and `to_integer` should work on std::string as well. Constructing a StringRef might make sense if we needed to perform some more complex operations (ones which std::string does not support) but I don't see anything like that here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116707