Re: [gdbserver/patch] Z packet support

[Daniel Jacobowitz <drow@false.org>]

On Wed, Jan 12, 2005 at 02:34:33PM +0100, Orjan Friberg wrote:
> Daniel Jacobowitz wrote:
> >
> >I'm fine with the approach.  The implementation needs a bit of work,
> >though.
> 
> There's a new ChangeLog entry (date change only) and patch at the end.  A 
> couple of comments to what you wrote first through:

You missed the most important problem with the previous posting, I'm
afraid - it's still whitespace mangled.  Please don't send patches
using format=flowed; it's very good for text, but lousy for data.
Either disable it or just attach them.  I've more or less figured out
the algorithm by which it ate your patch, but it's pretty tedious to
hand-merge; could you resend?

There's still some formatting glitches, but they're minor, I can just
correct as I edit.  I still want to add x86 support before approving
this so that I can test it.  The patch itself looks right, except:

> >>+	  addr = (*the_target->stopped_data_address) ();
> >>+
> >>+	  *buf++ = tohex ((addr >> 28) & 0xf);
> >>+	  *buf++ = tohex ((addr >> 24) & 0xf);
> >>+	  *buf++ = tohex ((addr >> 20) & 0xf);
> >>+	  *buf++ = tohex ((addr >> 16) & 0xf);
> >>+
> >>+	  *buf++ = tohex ((addr >> 12) & 0xf);
> >>+	  *buf++ = tohex ((addr >> 8) & 0xf);
> >>+	  *buf++ = tohex ((addr >> 4) & 0xf);
> >>+	  *buf++ = tohex (addr & 0xf);
> >>+
> >>+	  *buf++ = ';';
> >>+	}
> >
> >
> >This is broken on 64-bit targets.  This will require a certain amount
> >of shuffling to figure out the proper size of an address.
> 
> Living comfortably in a 32-bit world, I may have missed what you mean by "a 
> certain amount of shuffling".  I imagined a simple sizeof (addr) would do.

CORE_ADDR is always a long long in gdbserver, so your sizeof (addr)
probably doesn't work right for 32-bit targets.  I guess sizeof (void
*) is always right for this, though... at least for the kinds of
targets gdbserver supports now.

> >  - Some of the z/Z commands are breakpoints, not watchpoints.  Please
> >    only invoke watchpoint methods for watchpoint commands.
> 
> I assume you mean that I should exclude memory breakpoint and hardware 
> breakpoint.  Anyway, that's what I did.

Yep.

-- 
Daniel Jacobowitz