This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH v2 6/7] remote: consider addressable unit size when reading/writing memory
- From: Pedro Alves <palves at redhat dot com>
- To: Simon Marchi <simon dot marchi at ericsson dot com>, gdb-patches at sourceware dot org
- Date: Thu, 21 May 2015 18:48:00 +0100
- Subject: Re: [PATCH v2 6/7] remote: consider addressable unit size when reading/writing memory
- Authentication-results: sourceware.org; auth=none
- References: <1429127258-1033-1-git-send-email-simon dot marchi at ericsson dot com> <1429127258-1033-7-git-send-email-simon dot marchi at ericsson dot com>
On 04/15/2015 08:47 PM, Simon Marchi wrote:
> Adapt code in remote.c to take into account addressable unit size when
> reading/writing memory.
>
> A few variables are renamed and suffixed with _bytes or _units. This
> way, it's more obvious if there is any place where we add or compare
> values of different kinds (which would be a mistake).
You should put here some of the info you have on the series intro.
Even better in comments in the code even.
E.g., this example here:
> For RSP's m, M and X packets, the "length" parameters are used to
> correctly encode or decode the packet at a low level, where we don't
> want to have to deal with different target byte sizes. Also, for a
> network protocol, it would make sense to use a fixed-sized unit.
> Therefore, it would be easier if those lengths were always in bytes.
> Here is an example that corresponds to the previous MI example.
>
> -> $m1000,8#??
> <- aaaabbbbccccdddd
>
> -> $M1000,6:eeeeffffeeee#??
> <- OK
>
> -> $m1000,8#??
> <- eeeeffffeeeedddd
>
> --- a/gdb/common/rsp-low.c
> +++ b/gdb/common/rsp-low.c
> @@ -146,38 +146,64 @@ bin2hex (const gdb_byte *bin, char *hex, int count)
> return i;
> }
>
> +static int needs_escaping (gdb_byte b)
Line break before 'needs_escaping'. Missing intro comment.
> +{
> + return b == '$' || b == '#' || b == '}' || b == '*';
> +}
> +
> - if (output_index + 1 > out_maxlen)
> - break;
> - out_buf[output_index++] = b;
> + int idx = input_unit_index * unit_size + byte_index_in_unit;
> + gdb_byte b = buffer[idx];
> + if (needs_escaping(b))
Space before parens.
> + {
Otherwise looks good.
Thanks,
Pedro Alves