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

[Pedro Alves <palves@redhat.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