From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 17130 invoked by alias); 8 Dec 2008 07:35:15 -0000 Received: (qmail 17121 invoked by uid 22791); 8 Dec 2008 07:35:14 -0000 X-Spam-Check-By: sourceware.org Received: from main.gmane.org (HELO ciao.gmane.org) (80.91.229.2) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Mon, 08 Dec 2008 07:34:34 +0000 Received: from list by ciao.gmane.org with local (Exim 4.43) id 1L9adQ-00049Y-PF for gdb-patches@sources.redhat.com; Mon, 08 Dec 2008 07:34:28 +0000 Received: from lvk-gate.cmc.msu.ru ([212.192.248.233]) by main.gmane.org with esmtp (Gmexim 0.1 (Debian)) id 1AlnuQ-0007hv-00 for ; Mon, 08 Dec 2008 07:34:28 +0000 Received: from vladimir by lvk-gate.cmc.msu.ru with local (Gmexim 0.1 (Debian)) id 1AlnuQ-0007hv-00 for ; Mon, 08 Dec 2008 07:34:28 +0000 To: gdb-patches@sources.redhat.com From: Vladimir Prus Subject: Re: [patch] Simplify breakpoint.c function parameters Date: Mon, 08 Dec 2008 07:35:00 -0000 Message-ID: References: <20081207222253.GA6749@host0.dyn.jankratochvil.net> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7Bit User-Agent: KNode/0.10.9 X-IsSubscribed: yes 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: 2008-12/txt/msg00145.txt.bz2 Jan Kratochvil wrote: > Hi, > > the patch has no effect on the functionality but I find the simplified code > less magic to understand. > > It fixes a small bug - update_watchpoint() for bp_watchpoint (software > watchpoint) created breakpoint locations bp_loc_hardware_watchpoint update_watchpoint() is documented thusly: /* Assuming that B is a hardware watchpoint: - Reparse watchpoint expression, is REPARSE is non-zero - Evaluate expression and store the result in B->val - Update the list of values that must be watched in B->loc. If the watchpoint is disabled, do nothing. If this is local watchpoint that is out of scope, delete it. */ static void update_watchpoint (struct breakpoint *b, int reparse) but it appears to be actually called from breakpoint_re_set_one for the software watchpoint, too. I suspect it's a bug -- for software watchpoint we don't have to get a list of values to watch, which update_watchpoint() does more or less unconditionally. But this bug might be benign -- we probably never actually insert the computed locations. > while it > should be bp_loc_other. But I am not aware of any (possibly negative) effect > it had. > > No regressions on {x86_64,ia64}-unknown-linux-gnu. FWIW, this sounds reasonable and safe to me. - Volodya