[PATCH 3/3] gdb, gdbserver: introduce the 'x' RSP packet for binary memory read

[Tom Tromey]

>>>>> Tankut Baris Aktemur <tankut.baris.aktemur@intel.com> writes:

> Introduce an RSP packet, 'x', for reading from the remote server
> memory in binary format.  The binary write packet, 'X' already exists.
> The 'x' packet is essentially the same as 'm', except that the
> returned data is in binary format.  For transferring relatively large
> data (e.g.  shared library files), the 'x' packet can reduce the
> transfer costs.

Thanks for the patch.  I agree with the overall idea and the choice of
'x' as the packet.

> +@item x @var{addr},@var{length}
> +@anchor{x packet}
> +@cindex @samp{x} packet
> +Read @var{length} addressable memory units starting at address @var{addr}
> +(@pxref{addressable memory unit}).  Note that @var{addr} may not be aligned
> +to any particular boundary.

I think it would be good to address the "short read" request here:

    https://sourceware.org/bugzilla/show_bug.cgi?id=24751

that is, document what ought to happen in this case.


Now that 'E' must be quoted in binary packets, I think the "binary data"
section of the "Overview" node must be updated to mention that certain
replies -- and in particular this one -- must handle this properly.

> +  /* Determine which packet format to use.  */
> +  char packet_format = 'm';
> +DIAGNOSTIC_PUSH
> +DIAGNOSTIC_ERROR_SWITCH
> +  switch (m_features.packet_support (PACKET_x))
> +    {
> +    case PACKET_ENABLE:
> +      packet_format = 'x';
> +      break;
> +    case PACKET_DISABLE:
> +      packet_format = 'm';
> +      break;
> +    case PACKET_SUPPORT_UNKNOWN:
> +      internal_error (_("remote_read_bytes_1: bad internal state"));
> +    }
> +DIAGNOSTIC_POP

Can this possibly be done without the diagnostic stuff.

> +      /* Binary memory read support.  */
> +      strcat (own_buf, ";x+");

One thing I don't really know about the remote protocol is whether we
prefer to add advertised features (like this) or just have gdb probe.

> +	    suppress_next_putpkt_log ();

Ok, I guess I see why you did it this way now.  You can ignore that
earlier comment I guess.

Tom