Re: [PATCH v2 6/7] remote: consider addressable unit size when reading/writing memory

Pedro Alves
2015-05-21T17:48:07Z

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