From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 24085 invoked by alias); 30 Jan 2005 04:40:00 -0000 Mailing-List: contact gdb-patches-help@sources.redhat.com; run by ezmlm Precedence: bulk List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sources.redhat.com Received: (qmail 24049 invoked from network); 30 Jan 2005 04:39:55 -0000 Received: from unknown (HELO nevyn.them.org) (66.93.172.17) by sourceware.org with SMTP; 30 Jan 2005 04:39:55 -0000 Received: from drow by nevyn.them.org with local (Exim 4.43 #1 (Debian)) id 1Cv6sD-0006fI-Hc; Sat, 29 Jan 2005 23:39:45 -0500 Date: Sun, 30 Jan 2005 04:40:00 -0000 From: Daniel Jacobowitz To: Orjan Friberg Cc: gdb-patches@sources.redhat.com Subject: Re: [gdbserver/patch] Z packet support Message-ID: <20050130043944.GA25185@nevyn.them.org> Mail-Followup-To: Orjan Friberg , gdb-patches@sources.redhat.com References: <41ADDF6B.2040601@axis.com> <20041207023346.GA2524@nevyn.them.org> <41E52769.2030706@axis.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <41E52769.2030706@axis.com> User-Agent: Mutt/1.5.5.1+cvs20040105i X-SW-Source: 2005-01/txt/msg00301.txt.bz2 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