mib added inline comments.
================ Comment at: lldb/source/Core/Debugger.cpp:1409-1411 +static std::shared_ptr<LogHandler> +CreateLogHandler(LogHandlerKind log_handler_kind, int fd, bool should_close, + size_t buffer_size) { ---------------- Many (or most) arguments passed to this function might not be used depending on the kind of LogHandler you're instantiating. This is fine for now but if we add other handlers in the future that take different argument, it might become very confusing on which one to use. May be we could use a parameter pack with some template specialization to make this more robust ? ================ Comment at: lldb/source/Core/Debugger.cpp:1444 + CreateLogHandler(log_handler_kind, GetOutputFile().GetDescriptor(), + !should_close, buffer_size); } else { ---------------- I find it confusing to negate `should_close` but to initialize it to `true`. Since it’s not modified anywhere else, IMO it might be better to inline the value with a comment (`/*should_close=*/`), instead of using the variable. ================ Comment at: lldb/source/Core/Debugger.cpp:1466 + CreateLogHandler(log_handler_kind, (*file)->GetDescriptor(), + !should_close, buffer_size); m_stream_handlers[log_file] = log_handler_sp; ---------------- Ditto CHANGES SINCE LAST ACTION https://reviews.llvm.org/D128323/new/ https://reviews.llvm.org/D128323 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits