[Lldb-commits] [PATCH] D40636: Create a trivial lldb-test program

[Zachary Turner via lldb-commits]

You’re right that it’s basically reimplementing readobj but as you said, we
have our own object file readers. More importantly though, even if we
delegated to llvm, something could still in theory go wrong in the Module
class.

Plus, the important thing part of this patch is not this one module
command, but everything else
On Thu, Nov 30, 2017 at 2:31 AM Pavel Labath via Phabricator <
reviews at reviews.llvm.org> wrote:

> 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
>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20171130/de0cea1b/attachment.html>