JDevlieghere marked 11 inline comments as done.
JDevlieghere added inline comments.
================
Comment at: lldb/source/Commands/Options.td:436-437
Desc<"Set the log to be buffered, using the specified buffer size.">;
+ def log_handler : Option<"handler", "h">, Group<1>,
+ EnumArg<"Value", "LogHandlerType()">, Desc<"Use a custom handler.">;
def log_threadsafe : Option<"threadsafe", "t">, Group<1>,
----------------
clayborg wrote:
> Maybe "--type" or "--kind" would be better? Handler seems like an internal
> name. Maybe also mention what it will default to if not specified? Can the
> user see a list of the valid values or do we need to supply these in the
> description?
I think the concept of a "log handler" is sufficiently well known that it
should be easy for a a (power) user to understand. I'm concerned that "type"
and "kind" are a tad too high level, and therefore could easily be confused
with the log category and channel. For example, when someone asks to enable the
"gdb packet log", is that a category, channel, type or kind?
================
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) {
----------------
mib wrote:
> 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 ?
What did you have in mind? We don't know a-priori what the type will be. You
could template the function in the enum value, but you still wouldn't know
which arguments to pass before dispatching to the right function.
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
