[lldb-dev] [PATCH] new utility methods in GDBRemoteCommunicationClient

[Steve Pucci]

Thanks, Abid.  Answers to your questions below.

On Fri, Jan 24, 2014 at 2:11 AM, Abid, Hafiz <Hafiz_Abid at mentor.com> wrote:

>  Hi Steve,
>
> I had a quick look at the patch. It mostly looks good. Some small comments
> below.
>
>
>
> +    // For packets which specify a range of output to be returned,
>
> +    // return all of the output via a series of packets of the form
>
> +    // <prefix>:000,<size>
>
> +    // <prefix>:<size>,<size>
>
> +    // <prefix>:<size>*2,<size>
>
> +    // <prefix>:<size>*3,<size>
>
> +    // ...
>
> +    // until a "$l..." packet is received, indicating the end.
>
> +    // Concatenate the results together and return in response.
>
> I think that perhaps you can make this comment a bit more clear. Just
> separate what the remote target will do and what this function will do.
>

OK, will do, thanks.


>   +            printf("max packet size %lu\n", (unsigned
> long)m_max_packet_size);
>
> You forgot to remove this printf.
>

Right, thanks.


>   +        snprintf(sizeDescriptor, sizeof(sizeDescriptor), "%x,%x",
> offset, response_size);
>
> +        PacketResult result =
> SendPacketAndWaitForResponse((payload_prefix_str + sizeDescriptor).c_str(),
>
> +                                                           this_response,
>
> +
> /*send_async=*/false);
>
> When I see such packets in GDB remote log (e.g.
> $qXfer:features:read:64bit-core.xml:0,fff), there is a colon before offset.
> I don’t see it being added here. Is it supposed to be in payload_prefix_str?
>

Yes, that's the way I have it currently in my calling code.  It's been
tested in that environment and works but I'm not ready to check in the
calling code yet.  (I've also tested it in an environment without my
calling code to ensure this works as a standalone patch, though it's
unused).


> I suppose after this is approved, you would be working on parsing the xml
> that came from the gdbserver. Would it be only for shared libraries or you
> are also interested in xml target description?
>

Yes, my calling code parses using libxml2, and the plan is to have libxml2
conditionally compiled and linked in (using the same mechanism that clang
uses for the same purpose).  I have that working also in the calling code.

I considered putting the xml parser in GDBRemoteCommunicationClient also
but it seemed a bit higher-level than that class operates at.  But I'm open
to whatever people think here.  One possibility would be to split the
creation of the XML DOM and the walking of the DOM tree, and put the
creation of the DOM in GDBRemoteCommunicationClient.

We are also interested in the xml target description but it's not a
priority at the moment.

Thanks again for the review.  I'll send out the modified patch before
committing.

 - Steve


>
>
> Regards,
>
> Abid
>
>
>
>
>
>
>
>
>
> *From:* lldb-dev-bounces at cs.uiuc.edu [mailto:lldb-dev-bounces at cs.uiuc.edu]
> *On Behalf Of *Steve Pucci
> *Sent:* 23 January 2014 21:04
> *To:* lldb-dev at cs.uiuc.edu
> *Subject:* [lldb-dev] [PATCH] new utility methods in
> GDBRemoteCommunicationClient
>
>
>
> I'm working on better support for debugging against a remote stock
> gdbserver, and these are some new support methods I'm using.  I'm sending
> this separately for easier digestion.
>
>
>
> There are a couple of pieces:
>
>  * some lazy-evaluation members that store info listed in a qSupported
> response
>
>  * new method SendPacketsAndConcatenateResponses which is used for
> fetching fixed-size objects from the remote gdbserver by using multiple
> packets if necessary (first use will be to fetch shared-library XML files).
>
>
>
> Thanks,
>
>    Steve
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-dev/attachments/20140124/47bf57a0/attachment.html>