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
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
