[Lldb-commits] [PATCH] D128321: [lldb] Add OSLog log handler

[Jonas Devlieghere via Phabricator via lldb-commits]

JDevlieghere marked 2 inline comments as done.
JDevlieghere added inline comments.


================
Comment at: lldb/source/Host/common/Host.cpp:110
+#if !defined(__APPLE__)
+void Host::SystemLog(const std::string &message) {
+  fprintf(stderr, "%s", message.c_str());
----------------
labath wrote:
> labath wrote:
> > Why std::string? I'd expect StringRef (as that's what the LogHandler class uses), const char * (as that can avoid string copying sometimes) or a Twine (in case you want to be fancy), but a string seems like it combines the worst properties of all of these.
> Actually, I think we should just use a StringRef and use llvm::errs() for printing.
Right, I was optimizing for the Apple case where we need to go through a `std::string` to make sure it's null terminated. My reasoning was that if you pass in a StringRef you might end up with two copies if the original StringRef is already backed by a std::string.


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

https://reviews.llvm.org/D128321