clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.
So we either need to track categories for each buffered log message in the
circular buffer so that if the user specifies a category within a channel to
dump with "log dump lldb <category>" we can filter out the messages we dump if
they are not in the one or more categories that were supplied, or remove
categories from the "log dump" command and only specify the channel. Easy to
fix the code to work either way.
The idea is if the user does:
(lldb) log enable -b 5 -h circular lldb command state process
(lldb) ...
(lldb log dump lldb process
I would expect only "process" category messages to be shown. The current code
allows the user to specify categories, but they don't do anything. So we should
either obey the categories correctly by changing "LogHandler::Emit(StringRef
message)" to take an extra category mask like "LogHandler::Emit(StringRef
message, uint64_t category)" so that we can cache a vector of
"std::pair<std::string, uint64_t>" so we can emit only the requested categories
when using "log dump", or just remove the categories from the "log dump"
command.
================
Comment at: lldb/source/Commands/CommandObjectLog.cpp:359
+ CommandArgumentData channel_arg;
+ CommandArgumentData category_arg;
+
----------------
If we aren't going to correctly track what category that each log channel comes
from, then specifying the categories doesn't do anything and the categories
shouldn't be part of this command unless you want to fix that. We could easily
do this if we modify LogHandler::Emit(...) to take a category integer and then
our buffer would need to store category + message in an array.
================
Comment at: lldb/source/Utility/Log.cpp:239
+bool Log::DumpLogChannel(llvm::StringRef channel,
+ llvm::ArrayRef<const char *> categories,
+ llvm::raw_ostream &output_stream,
----------------
We don't need categories here. We have no way to separate each log message
since we don't store this, and the user shouldn't be required to know the
categories right?
================
Comment at: lldb/test/API/commands/log/basic/TestLogHandlers.py:20
+
+ self.runCmd("log enable -b 5 -h circular lldb commands")
+ self.runCmd("bogus", check=False)
----------------
Do we want to test that non circular buffers can't be dumped too to make sure
nothing goes wrong or that we get a good error message stating that this log
channel isn't circular?
================
Comment at: lldb/test/API/commands/log/basic/TestLogHandlers.py:22
+ self.runCmd("bogus", check=False)
+ self.runCmd("log dump lldb commands -f {}".format(self.log_file))
+
----------------
It seems wrong to specify the categories in the "log dump" command since we
won't do the right thing. If you can specified categories I would expect this
to work:
```
(lldb) log enable -b 5 -h circular lldb command state process
(lldb) ...
(lldb) log dump lldb process
```
And that only process log lines would come out. Since we don't store this
information, we shouldn't have to specify the categories at all, so the "log
dump" command should just take the channel:
```
(lldb) log dump lldb
```
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D128557/new/
https://reviews.llvm.org/D128557
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
