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

[Lirong Yuan via Phabricator via lldb-commits]

yuanzi marked 3 inline comments as done.
yuanzi added inline comments.


================
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);
----------------
labath wrote:
> 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?
I did that mainly to ensure the new code behaves exactly the same as the old. Since no complex functions are called, they should work interchangeably! Reverted to previous version.


================
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
----------------
labath wrote:
> 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.
Right! Done.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116707