From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 1624 invoked by alias); 24 Jun 2009 19:20:04 -0000 Received: (qmail 1610 invoked by uid 22791); 24 Jun 2009 19:20:03 -0000 X-SWARE-Spam-Status: No, hits=-2.5 required=5.0 tests=AWL,BAYES_00 X-Spam-Check-By: sourceware.org Received: from qnxmail.qnx.com (HELO qnxmail.qnx.com) (209.226.137.76) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 24 Jun 2009 19:19:58 +0000 Received: from Nebula.ott.qnx.com (nebula.ott.qnx.com [10.42.3.30]) by hub.ott.qnx.com (8.9.3/8.9.3) with ESMTP id PAA11541; Wed, 24 Jun 2009 15:19:50 -0400 Received: from [127.0.0.1] ([10.42.100.129]) by Nebula.ott.qnx.com with Microsoft SMTPSVC(6.0.3790.3959); Wed, 24 Jun 2009 15:19:56 -0400 Message-ID: <4A427C5B.8080205@qnx.com> Date: Wed, 24 Jun 2009 19:20:00 -0000 From: Aleksandar Ristovski User-Agent: Thunderbird 2.0.0.22 (Windows/20090605) MIME-Version: 1.0 To: Pedro Alves CC: gdb-patches@sourceware.org, Doug Evans Subject: Re: [patch] gdbserver: Add support for Z0/Z1 packets References: <200906231700.12402.pedro@codesourcery.com> <4A427584.2090908@qnx.com> <200906242011.16184.pedro@codesourcery.com> In-Reply-To: <200906242011.16184.pedro@codesourcery.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2009-06/txt/msg00658.txt.bz2 Pedro Alves wrote: > On Wednesday 24 June 2009 19:50:44, Aleksandar Ristovski wrote: >> Pedro Alves wrote: >>> On Tuesday 23 June 2009 16:17:58, Aleksandar Ristovski wrote: >>>> Pedro Alves wrote: >>>>> On Monday 22 June 2009 20:38:50, Aleksandar Ristovski wrote: >>>>> >>>>>>> Z0 and Z1 breakpoints also take a 'len' argument, just >>>>>>> like Z2-Z4. You should also pass those down. >>>>>>> >>>>>>> But, Let's take a step back --- why not just rename the >>>>>>> insert_watchpoint|remove_watchpoint functions to insert_point,remove_point, >>>>>>> and relax the type checks in server.c: >>>>>> But either way is fine with me - just let me know. >>>>> I'd prefer the approach I suggested, and worry about splitting >>>>> the breakpoints from watchpoints API if/when we actually need it. >>>>> >>>> Ok, then that version is committed. >>> Well, we had never seen "that" version >> Ok, to rectify this I am attaching two versions: one if I >> revert the changes I committed and the other is diff to what >> is in now. >> >> > ... and you bypassed the "rename" suggestion... >> >> I did not do any renaming - I think it is not terribly >> confusing since both in target.h comment and server.c 'Z' >> case it is made very clear that it handles both breakpoints >> and watchpoints (i.e. I don't find it any clearer if it was >> called "insert_point"... it would still require reading the >> comment in target.h) > > > Urgh, I was just about to press the send button when I saw > this message of yours. This version corrects a few troubles > with the previous commit (see ChangeLog) (one of them I still > see in your new patch) and I've tested it on x86_64-linux. > Aleksandar, please, please, do run the testsuite (and state > that you have) when posting patches. E.g., we could have caught > the vKill issue that Pierre fixed when we made only linux > report multi-process (the testsuite runs in "target remote" > mode, hence with multi-process off most of the way). > Your patch: Wasn't it more desireable to distinguish between (1) -unsupported and (-1)-error? i.e. in "default", shouldn't it be "res=-1" so that it makes it clear that gdb passed invalid argument? Testsuite: I do run it but I admit sometimes not due to issues my system is giving me lately; I have to run tests "manually" i.e. one by one (I'll have to take some time to figure out why is the test run, after some time, just starting to ERROR, unable to properly spawn new gdb instance). -- Aleksandar Ristovski QNX Software Systems