From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 31639 invoked by alias); 4 Jun 2008 07:31:20 -0000 Received: (qmail 31623 invoked by uid 22791); 4 Jun 2008 07:31:17 -0000 X-Spam-Check-By: sourceware.org Received: from zigzag.lvk.cs.msu.su (HELO zigzag.lvk.cs.msu.su) (158.250.17.23) by sourceware.org (qpsmtpd/0.31) with ESMTP; Wed, 04 Jun 2008 07:30:57 +0000 Received: from Debian-exim by zigzag.lvk.cs.msu.su with spam-scanned (Exim 4.63) (envelope-from ) id 1K3nSM-0002FN-RG for gdb-patches@sourceware.org; Wed, 04 Jun 2008 11:30:53 +0400 Received: from localhost ([127.0.0.1] helo=ip6-localhost) by zigzag.lvk.cs.msu.su with esmtps (TLS-1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.63) (envelope-from ) id 1K3nSE-0002F6-Ci; Wed, 04 Jun 2008 11:30:42 +0400 From: Vladimir Prus To: Nick Roberts Subject: Re: [PATCH:MI] Use observers for breakpoints Date: Wed, 04 Jun 2008 07:31:00 -0000 User-Agent: KMail/1.9.9 Cc: gdb-patches@sourceware.org References: <18498.3436.91443.361769@kahikatea.snap.net.nz> In-Reply-To: <18498.3436.91443.361769@kahikatea.snap.net.nz> MIME-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Content-Disposition: inline Message-Id: <200806041112.33746.ghost@cs.msu.su> 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-06/txt/msg00019.txt.bz2 On Sunday 01 June 2008 06:46:04 Nick Roberts wrote: > (gdb) > -break-watch i22 > =3Dbreakpoints-changed,bkpt=3D{number=3D"4",type=3D"watchpoint",disp=3D"k= eep",e >nabled=3D"y",addr=3D"",what=3D"i22",times=3D"0",original-location=3D"i22"} > ^done,wpt=3D{number=3D"4",exp=3D"i22"} Is this an output we actually want? ;-) With one breakpoint mentioned twice? > 2008-06-01 =A0Nick Roberts =A0 >=20 > =A0=A0=A0=A0=A0=A0=A0=A0* mi/mi-interp.c (mi_breakpoints_changed): New fu= nction. > =A0=A0=A0=A0=A0=A0=A0=A0(mi_interpreter_init): Register mi_breakpoints_ch= anged as > =A0=A0=A0=A0=A0=A0=A0=A0breakpoints_changed observer. >=20 > =A0=A0=A0=A0=A0=A0=A0=A0* mi/mi-cmd-break.c (breakpoint_notify, breakpoin= t_hooks): Delete. > =A0=A0=A0=A0=A0=A0=A0=A0(mi_cmd_break_insert): Don't use deprecated_set_g= db_event_hooks. >=20 > =A0=A0=A0=A0=A0=A0=A0=A0* breakpoint.c (condition_command, commands_comma= nd) > =A0=A0=A0=A0=A0=A0=A0=A0(commands_from_control_command, mention, delete_b= reakpoint) > =A0=A0=A0=A0=A0=A0=A0=A0(set_ignore_count, disable_breakpoint, do_enable_= breakpoint):=20 > =A0=A0=A0=A0=A0=A0=A0=A0Call observer_notify_breakpoints_changed. I wonder if you missed this bit in bpstat_stop_status: if (b->disposition =3D=3D disp_disable) { b->enable_state =3D bp_disabled; update_global_location_list (); } Probably, breakpoint_re_set_one should call the observer too. In fact, it's probably would be nice to add call to the observer to update_global_location_list. This function is called whenever we change any= property=20 of a breakpoint that affects what should be inserted in the target. The on= ly issue is that right now update_global_location_list does not know which bre= akpoint has changed -- we probably can add a parameter. This won't catch all cases -- there are properties of breakpoints that are = not=20 reflected in the target -- like condition, commands and ignore count. But s= eems like your patch has it covered already. Is there any way to make -break-insert report the new breakpoint directly, = as opposed to via notification? > Index: mi/mi-cmd-break.c > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > RCS file: /cvs/src/src/gdb/mi/mi-cmd-break.c,v > retrieving revision 1.19 > diff -p -u -r1.19 mi-cmd-break.c > --- mi/mi-cmd-break.c=A0=A0=A01 Feb 2008 16:24:46 -0000=A0=A0=A0=A0=A0=A0= =A01.19 > +++ mi/mi-cmd-break.c=A0=A0=A01 Jun 2008 02:17:21 -0000 > @@ -24,7 +24,6 @@ > =A0#include "breakpoint.h" > =A0#include "gdb_string.h" > =A0#include "mi-getopt.h" > -#include "gdb-events.h" > =A0#include "gdb.h" > =A0#include "exceptions.h" > =A0 > @@ -33,23 +32,6 @@ enum > =A0 =A0 =A0FROM_TTY =3D 0 > =A0 =A0}; > =A0 > -/* Output a single breakpoint. */ > - > -static void > -breakpoint_notify (int b) > -{ > - =A0gdb_breakpoint_query (uiout, b, NULL); > -} > - > - > -struct gdb_events breakpoint_hooks =3D > -{ > - =A0breakpoint_notify, > - =A0breakpoint_notify, > - =A0breakpoint_notify, > -}; > - > - > =A0enum bp_type > =A0 =A0{ > =A0 =A0 =A0REG_BP, > @@ -71,7 +53,6 @@ mi_cmd_break_insert (char *command, char > =A0 =A0char *condition =3D NULL; > =A0 =A0int pending =3D 0; > =A0 =A0struct gdb_exception e; > - =A0struct gdb_events *old_hooks; > =A0 =A0enum opt > =A0 =A0 =A0{ > =A0 =A0 =A0 =A0HARDWARE_OPT, TEMP_OPT /*, REGEXP_OPT */ , CONDITION_OPT, > @@ -131,8 +112,6 @@ mi_cmd_break_insert (char *command, char > =A0 =A0 =A0error (_("mi_cmd_break_insert: Garbage following ")); > =A0 =A0address =3D argv[optind]; > =A0 > - =A0/* Now we have what we need, let's insert the breakpoint! */ > - =A0old_hooks =3D deprecated_set_gdb_event_hooks (&breakpoint_hooks); > =A0 =A0/* Make sure we restore hooks even if exception is thrown. =A0*/ > =A0 =A0TRY_CATCH (e, RETURN_MASK_ALL) > =A0 =A0 =A0{ > @@ -164,7 +143,7 @@ mi_cmd_break_insert (char *command, char > =A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0=A0 = =A0_("mi_cmd_break_insert: Bad switch.")); > =A0=A0=A0=A0=A0=A0=A0=A0} > =A0 =A0 =A0} > - =A0deprecated_set_gdb_event_hooks (old_hooks); > + > =A0 =A0if (e.reason < 0) > =A0 =A0 =A0throw_exception (e); > =A0 > Index: mi/mi-interp.c > =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > RCS file: /cvs/src/src/gdb/mi/mi-interp.c,v > retrieving revision 1.30 > diff -p -u -r1.30 mi-interp.c > --- mi/mi-interp.c=A0=A0=A0=A0=A0=A03 May 2008 15:10:42 -0000=A0=A0=A0=A0= =A0=A0=A01.30 > +++ mi/mi-interp.c=A0=A0=A0=A0=A0=A01 Jun 2008 02:17:22 -0000 > @@ -68,6 +68,7 @@ static void mi_remove_notify_hooks (void > =A0 > =A0static void mi_new_thread (struct thread_info *t); > =A0static void mi_thread_exit (struct thread_info *t); > +static void mi_breakpoints_changed (int bnum); > =A0 > =A0static void * > =A0mi_interpreter_init (int top_level) > @@ -92,6 +93,7 @@ mi_interpreter_init (int top_level) > =A0 =A0 =A0{ > =A0 =A0 =A0 =A0observer_attach_new_thread (mi_new_thread); > =A0 =A0 =A0 =A0observer_attach_thread_exit (mi_thread_exit); > + =A0 =A0 =A0observer_attach_breakpoints_changed (mi_breakpoints_changed); > =A0 =A0 =A0} > =A0 > =A0 =A0return mi; > @@ -331,6 +333,27 @@ mi_thread_exit (struct thread_info *t) > =A0 =A0gdb_flush (mi->event_channel); > =A0} > =A0 > +static void > +mi_breakpoints_changed (int bnum) > +{ > + =A0struct mi_interp *mi =3D top_level_interpreter_data (); > + =A0struct interp *interp_to_use; > + =A0struct ui_out *old_uiout, *temp_uiout; > + =A0int version; > + > + =A0fprintf_unfiltered (mi->event_channel, "breakpoints-changed"); > + =A0interp_to_use =3D top_level_interpreter (); > + =A0old_uiout =3D uiout; > + =A0temp_uiout =3D interp_ui_out (interp_to_use); > + =A0version =3D mi_version (temp_uiout); > + =A0temp_uiout =3D mi_out_new (version); > + =A0uiout =3D temp_uiout; > + =A0breakpoint_query (bnum); Shall we have a helper function to do this creation of temporary uiout? I think that except for compatibility issues due to -break-insert no longer reporting the breakpoint that is created as part of ^done response, this patch is good. - Volodya