From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 879 invoked by alias); 31 Aug 2004 20:18:34 -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 808 invoked from network); 31 Aug 2004 20:18:30 -0000 Received: from unknown (HELO mx1.redhat.com) (66.187.233.31) by sourceware.org with SMTP; 31 Aug 2004 20:18:30 -0000 Received: from int-mx1.corp.redhat.com (int-mx1.corp.redhat.com [172.16.52.254]) by mx1.redhat.com (8.12.10/8.12.10) with ESMTP id i7VKIUS0023362 for ; Tue, 31 Aug 2004 16:18:30 -0400 Received: from pobox.toronto.redhat.com (pobox.toronto.redhat.com [172.16.14.4]) by int-mx1.corp.redhat.com (8.11.6/8.11.6) with ESMTP id i7VKIU306357; Tue, 31 Aug 2004 16:18:30 -0400 Received: from touchme.toronto.redhat.com (IDENT:postfix@touchme.toronto.redhat.com [172.16.14.9]) by pobox.toronto.redhat.com (8.12.8/8.12.8) with ESMTP id i7VKITse009891; Tue, 31 Aug 2004 16:18:29 -0400 Received: from redhat.com (toocool.toronto.redhat.com [172.16.14.72]) by touchme.toronto.redhat.com (Postfix) with ESMTP id 49BA9800214; Tue, 31 Aug 2004 16:18:29 -0400 (EDT) Message-ID: <4134DD15.2080806@redhat.com> Date: Tue, 31 Aug 2004 20:18:00 -0000 From: Jeff Johnston User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.4) Gecko/20030624 Netscape/7.1 MIME-Version: 1.0 To: Eli Zaretskii Cc: gdb-patches@sources.redhat.com Subject: Re: [RFA]: Change to_stopped_data_address ABI References: <4134C991.7050507@redhat.com> <01c48f92$Blat.v2.2.2$1eec0d00@zahav.net.il> In-Reply-To: <01c48f92$Blat.v2.2.2$1eec0d00@zahav.net.il> Content-Type: text/plain; charset=us-ascii; format=flowed Content-Transfer-Encoding: 7bit X-SW-Source: 2004-08/txt/msg00788.txt.bz2 Eli Zaretskii wrote: >>Date: Tue, 31 Aug 2004 14:55:13 -0400 >>From: Jeff Johnston >> >>The proposed change is to change the prototype to be: >> >>int >>to_stopped_data_address (CORE_ADDR *addr_p); >> >>If the input pointer is NULL, the function returns non-zero if it works on the >>given target, otherwise, it fails by returning 0. The function also returns 0 >>if unsuccessful. By separating out the success/fail code from the address, the >>new prototype allows for succeeding and returning any address, including 0. > > > Thanks. > > The idea is okay with me, but the code tells a bit different story > (unless I missed something, in which case I apologize). From your > description, I initially understood that you want to allow to return a > zero address when the watchpoint triggers at that address. For that, > if no watchpoint triggered, to_stopped_data_address will return zero > as its value, not put a NULL pointer into a place pointed to by its > argument. That would be okay with me, but your code does something > different: > Perhaps my description isn't clear enough. The function returns non-zero if successful (i.e. true). It returns 0 (false) to indicate failure or no stopped data address. It does not return an address at all; it is a true return code. The address is returned via the CORE_ADDR pointer argument. One can tell if the function works on a platform by passing a NULL pointer. The default function in target.c always returns 0 to indicate failure. > >>@@ -2739,8 +2739,7 @@ bpstat_stop_status (CORE_ADDR bp_addr, p >> struct value *v; >> int found = 0; >> >>- addr = target_stopped_data_address (); >>- if (addr == 0) >>+ if (!target_stopped_data_address (&addr)) >> continue; > > > This seems to say that target_stopped_data_address indeed returns a > zero value for the case where no watchpoint triggered... > > >>+int >>+i386_stopped_data_address (CORE_ADDR *addr_p) >> { >> CORE_ADDR addr = 0; >> int i; >> >>+ if (addr_p == NULL) >>+ return 1; >>+ >> dr_status_mirror = I386_DR_LOW_GET_STATUS (); >> >> ALL_DEBUG_REGISTERS(i) >>@@ -593,7 +598,16 @@ i386_stopped_data_address (void) >> if (maint_show_dr && addr == 0) >> i386_show_dr ("stopped_data_addr", 0, 0, hw_write); >> >>- return addr; >>+ *addr_p = addr; >>+ return 1; > > > ...but this returns 1 as the function's value and puts zero where the > argument points. Isn't that a contradiction? And doesn't this code > in i386_stopped_data_address still disallow support for a watchpoint > at address zero by retaining the previous magic meaning of a zero > address? Or did I miss something? > > >>+zero. When @var{addr_p} is non-NULL, return non-zero if the data address >>+for the triggered watchpoint is determined successfully, otherwise, return zero. > > > I think "watchpoint is determined successfully" is not clear enough. > Please rewrite the text to say exactly what does the zero value mean. > The intent is to tell a GDB hacker how to handle the case of zero > return value from target_stopped_data_address. >