labath added a comment.

I feel this is reimplementing llvm-readobj, but maybe that's appropriate as we 
are reimplementing object file readers as well (my original patch wouldn't be 
necessary if we were using the llvm object library). Minor comments below, but 
I actually quite like this approach. It's also much less code than I expected 
this will need.



================
Comment at: lldb/tools/lldb-test/CMakeLists.txt:3
+    (CMAKE_SYSTEM_NAME MATCHES "NetBSD" ))
+  # These targets do not have getopt support, so they rely on the one provided 
by
+  # liblldb. However, getopt is not a part of the liblldb interface, so we have
----------------
This is only scary if you link against liblldb, which you're not doing here. 
You also don't need to add lldbHost here, as it's already added in the list 
below. So this whole blob can just be deleted.


================
Comment at: lldb/tools/lldb-test/lldb-test.cpp:55
+      S->GetSectionData(Data);
+      llvm::outs() << "  Data: " << Data.GetCStr(&Offset) << "\n\n";
+    }
----------------
My test case is probably the only place where this will actually be a real C 
string. I think we should go with something more generic like hex-encoding the 
data.


https://reviews.llvm.org/D40636



_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to