From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 16625 invoked by alias); 3 Jul 2005 17:03:01 -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 16613 invoked by uid 22791); 3 Jul 2005 17:02:57 -0000 Received: from nevyn.them.org (HELO nevyn.them.org) (66.93.172.17) by sourceware.org (qpsmtpd/0.30-dev) with ESMTP; Sun, 03 Jul 2005 17:02:57 +0000 Received: from drow by nevyn.them.org with local (Exim 4.51) id 1Dp7rr-0003sd-Lc; Sun, 03 Jul 2005 13:02:55 -0400 Date: Sun, 03 Jul 2005 17:03:00 -0000 From: Daniel Jacobowitz To: Nick Roberts Cc: Bob Rossi , gdb-patches@sources.redhat.com Subject: Re: [PATCH] Hooks still needed for annotations Message-ID: <20050703170255.GD13811@nevyn.them.org> Mail-Followup-To: Nick Roberts , Bob Rossi , gdb-patches@sources.redhat.com References: <17053.24737.153388.915345@farnswood.snap.net.nz> <20050601113004.GC15414@white> <17054.10607.109160.333076@farnswood.snap.net.nz> <20050603190856.GB32722@nevyn.them.org> <17056.56022.36723.292491@farnswood.snap.net.nz> <20050603235923.GA9992@nevyn.them.org> <17057.7583.990091.951816@farnswood.snap.net.nz> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <17057.7583.990091.951816@farnswood.snap.net.nz> User-Agent: Mutt/1.5.8i X-SW-Source: 2005-07/txt/msg00021.txt.bz2 On Sat, Jun 04, 2005 at 03:18:55PM +1200, Nick Roberts wrote: > > The way to prevent them from being removed again is to add test cases. > > OK, I've done this. Running the tests revealed a small bug with > "set annotate" -- it didn't call _initialize_annotate to set the hooks. > This must be why the tests didn't fail when the hooks were removed in the > first place. > > I've only tested for breakpoints-invalid annotations when breakpoints > have been deleted as I could simply adapt existing tests. They should > also be present when breakpoints are enabled/disabled. However, this test > should be sufficient to prevent the inadvertant removal of the hooks. OK, having answered the rest of the side topics in this discussion I now come to the actual patch... Having reread the discussion I would like to ask you what your goal is with this patch. You don't use this annotation, and you've said that it is very awkward to use because of the amount of output it produces. Why should we fix it (as opposed to garbage collecting it) if no one has missed it? About the patch: > *** /home/nick/src/gdb/event-top.c.~1.40.~ 2005-03-17 12:23:51.000000000 +1300 > --- /home/nick/src/gdb/event-top.c 2005-06-04 14:36:37.000000000 +1200 > *************** > *** 43,48 **** > --- 43,50 ---- > /* readline defines this. */ > #undef savestring > > + extern void _initialize_annotate (void); > + > static void rl_callback_read_char_wrapper (gdb_client_data client_data); > static void command_line_handler (char *rl); > static void command_line_handler_continuation (struct continuation_arg *arg); No extern prototypes. If you need to call a function from another file, it should be defined in a header, and the header should be included in both the defining and using files. That's the only way in C we can make sure that argument mismatch does not creep in. Also, please move this into a new function if you want it to be called other than from init.c; the _initialize_* functions are magic. > *** /home/nick/src/gdb/interps.c.~1.15.~ 2005-04-26 21:44:15.000000000 +1200 > --- /home/nick/src/gdb/interps.c 2005-06-04 15:12:16.000000000 +1200 > *************** > *** 326,333 **** > deprecated_query_hook = 0; > deprecated_warning_hook = 0; > deprecated_create_breakpoint_hook = 0; > - deprecated_delete_breakpoint_hook = 0; > - deprecated_modify_breakpoint_hook = 0; > deprecated_interactive_hook = 0; > deprecated_registers_changed_hook = 0; > deprecated_readline_begin_hook = 0; > --- 326,331 ---- deprecated_create_breakpoint_hook is now set nowhere. deprecated_delete_breakpoint_hook and deprecated_modify_breakpoint_hook are set only by annotations. There's a total of three call sites between the two hooks. And each of those takes up more screen real estate than a call to breakpoints_changed, which is what they eventually become. The comments in mention() suggest that at one time, GDB was trying to move away from breakpoints_changed to a more specific set of hooks. But now the hooks are more or less dead, and to get full mileage out of them they're going to need a redesign. So maybe we should just delete all three hooks, and replace the two that annotations use with calls to breakpoints_changed. -- Daniel Jacobowitz CodeSourcery, LLC