Re: [rfc/rfa] [3/4] SPU enhancements: gdbserver support

[Daniel Jacobowitz <drow at false dot org>]

On Sat, Jun 02, 2007 at 09:33:29PM +0200, Ulrich Weigand wrote:
> Hello,
> 
> this patch, implemented by Markus Deuling, extends TARGET_OBJECT_SPU
> support to remote gdbserver debugging.
> 
> This is done by extended the remote protocol by two new packet types,
> qXfer:spu:read and qXfer:spu:write.  In addition, the patch contains
> the following changes to remote packet support:

The TARGET_OBJECT_SPU support looks fine.  I don't really like either
of these changes, though.

> - It adds a "length" argument to qXfer::write type packets.  This is
>   to make parsing of the packet easier, and to bring it in line with
>   the 'X' packet format.  (It also provides a bit of extra redundancy
>   to detect transmission problems.)

It adds another case that stubs need to check for (length != length of
supplied data), and it's redundant.  I have some code already written
to generate and parse binary packets without a specified length, if
you'd like me to post it.  It's for the project I'm going to submit
once I finish with shared library lists - file transfer through
gdbserver.

I think more redundancy is not what we need in an already chatty
protocol :-)  And anyone using this to debug an SPU is probably going
to do so over a reliable link (pipe or TCP socket).

> - The remote_read_qxfer attempts to cache received end-of-object
>   packets.  However, this is a problem for some spufs objects as
>   they can start out with no content (length zero), and acquire
>   actual content later on.  If remote_read_qxfer has already cache
>   an end-of-object at lenght zero, future re-reads of the object
>   will always return zero even if the object by now has actual
>   content.  To fix this we've added a CACHEABLE flag to the
>   function and use it to disable the cache for spufs objects.

How about this simpler fix?

-  if (rs->buf[0] == 'l')
+  if (rs->buf[0] == 'l' && offset + i > 0)

It's only supposed to trigger for the next part of the same read
operation.  If we got no bytes, then the operation is already over.

-- 
Daniel Jacobowitz
CodeSourcery