[Lldb-commits] [PATCH] D68434: SBFile support in SBCommandReturnObject
Pavel Labath via Phabricator via lldb-commits
lldb-commits at lists.llvm.org
Wed Oct 9 07:21:10 PDT 2019
labath added a comment.
Some nits inline, otherwise it looks fine to me.
================
Comment at: lldb/include/lldb/Utility/ReproducerInstrumentation.h:243
+template <class T> struct serializer_tag {
+ typedef typename std::conditional<std::is_trivially_copyable<T>::value, ValueTag, NotImplementedTag>::type type;
+};
----------------
clang-format this
================
Comment at: lldb/source/API/SBCommandReturnObject.cpp:118-142
size_t SBCommandReturnObject::PutOutput(FILE *fh) {
LLDB_RECORD_METHOD(size_t, SBCommandReturnObject, PutOutput, (FILE *), fh);
-
if (fh) {
size_t num_bytes = GetOutputSize();
if (num_bytes)
return ::fprintf(fh, "%s", GetOutput());
}
----------------
Have these methods delegate to one another (I'm not sure which should be the "base" one TBH).
================
Comment at: lldb/source/API/SBCommandReturnObject.cpp:133
+ return 0;
+ return file_sp->Printf("%s", GetOutput());
+}
----------------
Why not just `file_sp->Write(GetOutput(), GetOutputSize())` ? It invoking formatting function for this seems overkill (and ideally I would like to remove formatted output capabilities from the file class completely).
================
Comment at: lldb/source/API/SBCommandReturnObject.cpp:275-284
+ FileSP file = std::make_shared<NativeFile>(fh, transfer_ownership);
+ ref().SetImmediateOutputFile(file);
}
void SBCommandReturnObject::SetImmediateErrorFile(FILE *fh,
bool transfer_ownership) {
LLDB_RECORD_METHOD(void, SBCommandReturnObject, SetImmediateErrorFile,
----------------
Here the need isn't as pressing, but I'd probably have these delegate too.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D68434/new/
https://reviews.llvm.org/D68434
More information about the lldb-commits
mailing list