[Lldb-commits] [PATCH] D116005: [lldb] [Process/FreeBSDKernel] Introduce libkvm support

Pavel Labath via Phabricator via lldb-commits lldb-commits at lists.llvm.org
Wed Dec 22 03:48:27 PST 2021

labath added a comment.

Just a couple of quick comments.

In D116005#3203077 <https://reviews.llvm.org/D116005#3203077>, @mgorny wrote:

> In D116005#3202721 <https://reviews.llvm.org/D116005#3202721>, @labath wrote:
>> How about factoring the two implementations into subclasses. If the classes are small, maybe you can just declare them in the cpp file...
> Yes, that's a nice idea. As an extra, we can easily use the real pointer types and remove all these casts.

Yep. :)

Comment at: lldb/source/Plugins/Process/FreeBSDKernel/ProcessFreeBSDKernel.cpp:29-63
+class ProcessFreeBSDKernelFVC : public ProcessFreeBSDKernel {
+  ProcessFreeBSDKernelFVC(lldb::TargetSP target_sp, lldb::ListenerSP listener,
+                          fvc_t *fvc);
+  ~ProcessFreeBSDKernelFVC();
put into anonymous namespace, for maximum compliance.

Comment at: lldb/source/Plugins/Process/FreeBSDKernel/ProcessFreeBSDKernel.cpp:58
+  const char *GetError();
is looks like this ought to be private

Comment at: lldb/source/Plugins/Process/FreeBSDKernel/ProcessFreeBSDKernel.cpp:86
+#if defined(__FreeBSD__)
+    kvm_t *kvm =
+        kvm_open2(executable->GetFileSpec().GetPath().c_str(),
Do you really want to do this if the fvc call was successful? What is the expected priority of the two implementations? Maybe just use return statements instead of assignments?



More information about the lldb-commits mailing list