[Lldb-commits] [PATCH] D31374: Add support for tracing hello-world application on NetBSD

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

I wasn't able to go into this too deeply, but here is what I have after a quick pass. I won't be able to review this thoroughly that soon, but I think it can go in after you take my comments into consideration.



================
Comment at: source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp:450
+  Log *log(ProcessPOSIXLog::GetLogIfAllCategoriesSet(POSIX_LOG_PROCESS));
+  LLDB_LOG(log, "selecting running thread for interrupt target");
+
----------------
this comment does not make sense in this context


================
Comment at: source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp:620
+                                          lldb::addr_t &addr) {
+  return Error();
+}
----------------
This will return a success value. You probably wan't `return Error("Unimplemented");` or something like that.


================
Comment at: source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp:624
+Error NativeProcessNetBSD::DeallocateMemory(lldb::addr_t addr) {
+  return Error();
+}
----------------
same here


================
Comment at: source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp:666
+                                                   FileSpec &file_spec) {
+  FileSpec module_file_spec(module_path, true);
+
----------------
`return Error("Unimplemented");`


================
Comment at: source/Plugins/Process/NetBSD/NativeProcessNetBSD.cpp:801
+  int status;
+  ::pid_t wait_pid = waitpid(WAIT_ANY, &status, WALLSIG | WNOHANG);
+
----------------
you could probably use process id instead of WAIT_ANY. The reason we needed -1 on linux is because each thread is reported separately.


================
Comment at: source/Plugins/Process/NetBSD/NativeThreadNetBSD.cpp:32
+  Log *log(GetLogIfAllCategoriesSet(LIBLLDB_LOG_THREAD));
+  if (log)
+    log->Printf("NativeThreadNetBSD::%s called with signal 0x%02" PRIx32,
----------------
How about using LLDB_LOG here?


================
Comment at: source/Plugins/Process/NetBSD/NativeThreadNetBSD.cpp:142
+                                        uint32_t watch_flags, bool hardware) {
+  return Error();
+}
----------------
"Unimplemented" (and below as well).


Repository:
  rL LLVM

https://reviews.llvm.org/D31374