labath added a comment. Just a couple of comments to make this code more llvm-y. Without those, we have just moved the place which constructs the temporary std::string. I haven't highlighted all the uses, but overall I'd recommend using the `Format` function or something similar instead of `Printf`, as the latter can be error-prone (because of the need to pass the right PRIx macro to get the integer size right) and/or inefficient (need to do the `.str().c_str()` dance on StringRefs).
================ Comment at: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:1808 + response.PutCString("encoding:"); + response.PutCString(encoding.str().c_str()); + response.PutChar(';'); ---------------- Despite the name, PutCString actually takes a StringRef, so you can just drop the `.str().c_str()` thingy. In fact, I'd consider just writing `response << "encoding:" << encoding << ';';` ================ Comment at: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:2814 + if (!encoding.empty()) + response.Printf("encoding=\"%s\" ", encoding.str().c_str()); + ---------------- Similarly, `response << "encoding='" << encoding << "' "`, or `response.Format("encoding='{0}'", encoding)` would be shorter, and avoid string copying. ================ Comment at: lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerLLGS.cpp:2906 + return BuildTargetXml(); + } + ---------------- no braces CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74217/new/ https://reviews.llvm.org/D74217 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits