This is the mail archive of the insight@sources.redhat.com mailing list for the Insight project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]

Re: [RFA] New Event Model - Part B


Keith,

Most of this looks fine to me.  I am not quite sure why you use the get 
method of the Event object instead of using its variables - seems a bit 
awkward, but the use is okay.   Since you would have to pay me in 
something more precious than gold to look over SrcTextWin code - so I 
only scanned that briefly, but the rest - and the discussion - seemed 
good.

Jim

On Wednesday, April 18, 2001, at 03:52 PM, Keith Seitz wrote:

> Well, let's overload your brains. :-)
>
> Here's how we will end up using this stuff. I've implemented the
> BreakpointEvent (and maybe the TracepointEvent) in the GUI. This 
> required
> some reworking of some of the basics...
>
> The biggest changes (should) occur in interface.tcl, but, unfortunately,
> the (current) event system is so wrong, that we will be changing lots of
> other things. BreakpointEvent is a good (and painful) example.
>
> When the BreakpointEvent is created (in gdbtk_tcl_breakpoint),
> gdb_get_breakpoint_info is called to get all information about the 
> event.
> GDBEvent::get may be used later in event handlers to get at this data
> (see Part A & gdbevent.ith for an explanation of all the members of the
> returned array/list).
>
> The current event notification (for BPs) is completely wrong. MI did 
> this
> right: all a gui needs is the action (create, delete, modify) and the
> handle gdb uses for the BP (a number).  I changed the breakpoint stuff 
> to
> support this model (so we can, hypotethically whack 
> create_breakpoint_hook
> and friends in favor of a gdb_event dispatch).
>
> The most unusual change occurs as a result of this "wrong" notification
> model when one deletes breakpoints. gdbtk_tcl_breakpoint gets "delete" 
> and
> the breakpoint number, attempts to instantiate a new BreakpointEvent
> object, and *fails*, because gdb_get_breakpoint_info could not find the
> breakpoint.
>
> Our (old and new) event models presume that you will have access to the
> breakpoint just before it is deleted. This is okay, though:
> delete_breakpoint_hook is called BEFORE the breakpoint is deleted. MI 
> got
> this right, too.
>
> The problem is that clear_command takes the breakpoint off of
> breakpoint_chain BEFORE the hook is called (and, consequently, before
> the BreakpointEvent is created). So when the BreakpointEvent constructor
> calls "gdb_get_breakpoint_info", it fails to find the breakpoint.
>
> delete_breakpoint, on the other hand, takes the breakpoint off the chain
> AFTER it deletes the breakpoint (so you can call 
> gdb_get_breakpoint_info on
> it in the hook). As you can guess, we really want to use 
> delete_command, not
> clear_command. I had to rewrite some bis of SrcTextWin to handle this. 
> This
> is why you'll see me change from gdb_cmd "clear FOO" to gdb_cmd "delete
> $number". (Of course this is just one simple step from getting rid of
> "gdb_cmd", but this patch is already complicated enough.)
>
> Here's the complete patches to interface.tcl, bpwin.it[hb],
> srctextwin.it[hb], gdbtk-cmds.c, and gdbtk-hooks.c. (One small note: I
> fixed a slew of bugs in SrcTextWin regarding duplicate breakpoints. As
> written, the code only worked on SOURCE mode windows. It now works on
> ASSEMBLY, MIXED, and SRC+ASM, too.) My comments are marked with "%%%%%".
>
> This is a lot to digest, sorry.
>
> Index: interface.tcl
> ===================================================================
> RCS file: /cvs/src/src/gdb/gdbtk/library/interface.tcl,v
> retrieving revision 1.17
> diff -p -u -r1.17 interface.tcl
> --- interface.tcl	2001/04/18 17:44:00	1.17
> +++ interface.tcl	2001/04/18 22:22:40
> @@ -1,5 +1,5 @@
>  # Interface between GDB and Insight.
> -# Copyright 1997, 1998, 1999 Cygnus Solutions
> +# Copyright 1997, 1998, 1999, 2001 Red Hat, Inc.
>  #
>  # This program is free software; you can redistribute it and/or modify 
> it
>  # under the terms of the GNU General Public License (GPL) as published 
> by
>
> %%%%% Update copyright. :-)
>
> @@ -16,9 +16,10 @@
>  global gdbtk_state
>  set gdbtk_state(busyCount) 0
>
> +# *** DEPRECATED: Use GDBEventHandler::breakpoint instead.
>  # This is run when a breakpoint changes.  The arguments are the
>  # action, the breakpoint number, and the breakpoint info.
> -define_hook gdb_breakpoint_change_hook
> +#define_hook gdb_breakpoint_change_hook
>
>  # This is run when a `set' command successfully completes in gdb.  The
>  # first argument is the gdb variable name (as a Tcl list).  The second
>
> %%%%% Deprecate (remove) gdb_breakpoint_changed_hook.
>
> @@ -443,17 +444,21 @@ proc gdbtk_tcl_end_variable_annotation {
>  # ------------------------------------------------------------------
>  # PROC: gdbtk_tcl_breakpoint -
>  # ------------------------------------------------------------------
> -proc gdbtk_tcl_breakpoint {action bpnum addr line file bp_type enabled 
> thread} {
> -#  debug "BREAKPOINT: $action $bpnum $addr $line $file $bp_type 
> $enabled $thread "
> -  run_hooks gdb_breakpoint_change_hook $action $bpnum $addr $line 
> $file $bp_type $enabled $thread
> +proc gdbtk_tcl_breakpoint {action bpnum} {
> +#  debug "BREAKPOINT: $action $bpnum"
> +  set e [BreakpointEvent \#auto -action $action -number $bpnum]
> +  GDBEventHandler::dispatch $e
> +  delete object $e
>  }
>
>  # ------------------------------------------------------------------
>  # PROC: gdbtk_tcl_tracepoint -
>  # ------------------------------------------------------------------
> -proc gdbtk_tcl_tracepoint {action tpnum addr line file pass_count} {
> -#  debug "TRACEPOINT: $action $tpnum $addr $line $file $pass_count"
> -  run_hooks gdb_breakpoint_change_hook $action $tpnum $addr $line 
> $file tracepoint
> +proc gdbtk_tcl_tracepoint {action tpnum} {
> +#  debug "TRACEPOINT: $action $tpnum"
> +  set e [TracepointEvent \#auto -action $action -number $tpnum]
> +  GDBEventHandler::dispatch $e
> +  delete object $e
>  }
>
>  # ------------------------------------------------------------------
>
> %%%% gdbtk_tcl_breakpoint now takes only two args, creates an event 
> based
> on that info, and dispatches the event.
>
> Index: bpwin.ith
> ===================================================================
> RCS file: /cvs/src/src/gdb/gdbtk/library/bpwin.ith,v
> retrieving revision 1.1.1.1
> diff -p -u -r1.1.1.1 bpwin.ith
> --- bpwin.ith	2000/02/07 00:19:42	1.1.1.1
> +++ bpwin.ith	2001/04/18 22:22:46
> @@ -1,5 +1,5 @@
> -# Breakpoint window class definition for GDBtk.
> -# Copyright 1997, 1998, 1999 Cygnus Solutions
> +# Breakpoint window class definition for Insight
> +# Copyright 1997, 1998, 1999, 2001 Red Hat, Inc.
>  #
>  # This program is free software; you can redistribute it and/or modify 
> it
>  # under the terms of the GNU General Public License (GPL) as published 
> by
>
> @@ -26,13 +26,15 @@ class BpWin {
>      method bp_restore {}
>      method bp_store {}
>      method bp_type { i }
> -    method update {action bpnum addr {linenum {}} {file {}} {type 0} 
> args}
>      method bp_all { command }
>      method get_actions {bpnum}
>      method toggle_threads {}
>      method reconfig {}
>      method goto_bp {r}
>
> +    # GDB Events
> +    method breakpoint {event}
> +    method tracepoint {event}
>    }
>
>    private {
>
> %%%%% "update" was renamed to "breakpoint". It does everything we need.
> (Besides I expect to use update for some other event...)
>
> @@ -48,9 +50,9 @@ class BpWin {
>      variable show_threads	;#cached copy of [pref get 
> gdb/bp/show_threads]
>
>      method build_win {}
> -    method bp_add { bpnum {tracepoint 0}}
> -    method bp_modify { bpnum {tracepoint 0} }
> -    method bp_delete { bpnum }
> +    method bp_add {bp_event {tracepoint 0}}
> +    method bp_modify {bp_event {tracepoint 0}}
> +    method bp_delete {bp_event}
>    }
>
>  }
>
> %%%%% Change bpwin's helper funcs to use the event instead of querying
> for the data again. (The event contains a single, read-only copy of the
> data.)
>
> Index: bpwin.itb
> ===================================================================
> RCS file: /cvs/src/src/gdb/gdbtk/library/bpwin.itb,v
> retrieving revision 1.4
> diff -p -u -r1.4 bpwin.itb
> --- bpwin.itb	2000/12/07 20:14:02	1.4
> +++ bpwin.itb	2001/04/18 22:22:54
> @@ -1,5 +1,5 @@
>  # Breakpoint window for GDBtk.
> -# Copyright 1997, 1998, 1999 Cygnus Solutions
> +# Copyright 1997, 1998, 1999, 2001 Red Hat, Inc.
>  #
>  # This program is free software; you can redistribute it and/or modify 
> it
>  # under the terms of the GNU General Public License (GPL) as published 
> by
>
> @@ -18,7 +18,6 @@
>  body BpWin::constructor {args} {
>    window_name "Breakpoints" "BPs"
>
> -  add_hook gdb_breakpoint_change_hook "$this update"
>    if {[pref getd gdb/bp/menu] != ""} {
>      set mbar 0
>    }
>
> %%%%% Bye-bye hook!
>
> @@ -32,9 +31,7 @@ body BpWin::constructor {args} {
>  # ------------------------------------------------------------------
>  #  DESTRUCTOR:  destroy the breakpoint window
>  # ------------------------------------------------------------------
> -body BpWin::destructor {} {
> -  remove_hook gdb_breakpoint_change_hook "$this update"
> -}
> +body BpWin::destructor {} {}
>
>
>  # ------------------------------------------------------------------
>
> %%%%% No need to do this anymore.
>
> @@ -168,12 +165,16 @@ body BpWin::build_win {} {
>    if { $tracepoints == 0 } {
>      # insert all breakpoints
>      foreach i [gdb_get_breakpoint_list] {
> -      bp_add $i
> +      set e [BreakpointEvent \#auto -action create -number $i]
> +      bp_add $e
> +      delete object $e
>      }
>    } else {
>      # insert all tracepoints
>      foreach i [gdb_get_tracepoint_list] {
> -      bp_add $i 1
> +      set e [TracepointEvent \#auto -action create -number $i]
> +      bp_add $e 1
> +      delete object $e
>      }
>    }
>
> %%%%% One unfortunate side affect... Need to create events to insert
> them. It's really no big deal, though, since it does almost the same
> amount of work as before.
>
> @@ -185,28 +186,23 @@ body BpWin::build_win {} {
>  # ------------------------------------------------------------------
>  #  METHOD:  bp_add - add a breakpoint entry
>  # ------------------------------------------------------------------
> -body BpWin::bp_add { bpnum {tracepoint 0}} {
> +body BpWin::bp_add {bp_event {tracepoint 0}} {
>    global _bp_en _bp_disp tcl_platform _files
> -
> +
> +  array set event [$bp_event get]
>    if {$tracepoint} {
> -    set bpinfo [gdb_get_tracepoint_info $bpnum]
> -    lassign $bpinfo file func line pc enabled pass_count \
> -      step_count thread hit_count actions
> -    set disposition tracepoint
> +    array set event [list disposition tracepoint]
>      set bptype tracepoint
>    } else {
> -    set bpinfo [gdb_get_breakpoint_info $bpnum]
> -    lassign $bpinfo file func line pc type enabled disposition \
> -      ignore_count commands cond thread hit_count
>      set bptype breakpoint
>    }
>
> %%%%% No need to call "gdb_get_breakpoint_info" -- it's already been
> done! :-)
>
> A lot of what follows is just changing "foo" -> "event(foo)", i.e.,
> pulling the data from the event.
>
> -  debug "bp_add bpnum=$bpnum thread=$thread show=$show_threads"
> +  debug "bp_add bpnum=$event(number) thread=$event(thread) 
> show=$show_threads"
>    set i $next_row
> -  set _bp_en($i) $enabled
> -  set _bp_disp($i) $disposition
> +  set _bp_en($i) $event(enabled)
> +  set _bp_disp($i) $event(disposition)
>    set temp($i) ""
> -  switch $disposition {
> +  switch $event(disposition) {
>      donttouch { set color [pref get gdb/src/bp_fg] }
>      delete {
>        set color [pref get gdb/src/temp_bp_fg]
> @@ -218,7 +214,7 @@ body BpWin::bp_add { bpnum {tracepoint 0
>      default { set color yellow }
>    }
>
> -  if {$thread != "-1"} {set color [pref get gdb/src/thread_fg]}
> +  if {$event(thread) != "-1"} {set color [pref get gdb/src/thread_fg]}
>
>    if {$tcl_platform(platform) == "windows"} {
>      checkbutton $twin.en$i -relief flat -variable _bp_en($i) -bg $bg1 \
> @@ -229,22 +225,22 @@ body BpWin::bp_add { bpnum {tracepoint 0
>    }
>
>    if {$tracepoints} {
> -    label $twin.num$i -text "$bpnum " -relief flat -anchor e -font 
> src-font -bg $bg1
> +    label $twin.num$i -text "$event(number) " -relief flat -anchor e 
> -font src-font -bg $bg1
>    }
> -  label $twin.addr$i -text "$pc " -relief flat -anchor e -font 
> src-font -bg $bg1
> -  if {[info exists _files(short,$file)]} {
> -    set file $_files(short,$file)
> +  label $twin.addr$i -text "$event(address) " -relief flat -anchor e 
> -font src-font -bg $bg1
> +  if {[info exists _files(short,$event(file))]} {
> +    array set event [list file $_files(short,$event(file))]
>    } else {
>      # FIXME.  Really need to do better than this.
> -    set file [::file tail $file]
> +    array set event [list file [::file tail $event(file)]]
>    }
>    if {$show_threads} {
> -    if {$thread == "-1"} {set thread "ALL"}
> -    label $twin.thread$i -text "$thread " -relief flat -anchor e -font 
> src-font -bg $bg1
> +    if {$event(thread) == "-1"} {array set event [list thread "ALL"]}
> +    label $twin.thread$i -text "$event(thread) " -relief flat -anchor 
> e -font src-font -bg $bg1
>    }
> -  label $twin.file$i -text "$file " -relief flat -anchor e -font 
> src-font -bg $bg1
> -  label $twin.line$i -text "$line " -relief flat -anchor e -font 
> src-font -bg $bg1
> -  label $twin.func$i -text "$func " -relief flat -anchor e -font 
> src-font -bg $bg1
> +  label $twin.file$i -text "$event(file) " -relief flat -anchor e 
> -font src-font -bg $bg1
> +  label $twin.line$i -text "$event(line) " -relief flat -anchor e 
> -font src-font -bg $bg1
> +  label $twin.func$i -text "$event(function) " -relief flat -anchor e 
> -font src-font -bg $bg1
>
>    if {$tracepoints} {
>      label $twin.pass$i -text "$pass_count " -relief flat -anchor e 
> -font src-font -bg $bg1
> @@ -276,7 +272,7 @@ body BpWin::bp_add { bpnum {tracepoint 0
>    # This used to be the last row.  Fix it vertically again.
>    grid rowconfigure $twin $i -weight 0
>
> -  set index_to_bpnum($i) $bpnum
> +  set index_to_bpnum($i) $event(number)
>    set Index_to_bptype($i) $bptype
>    incr i
>    set next_row $i
> @@ -417,25 +413,20 @@ body BpWin::bp_select { r } {
>  # ------------------------------------------------------------------
>  #  METHOD:  bp_modify - modify a breakpoint entry
>  # ------------------------------------------------------------------
> -body BpWin::bp_modify { bpnum {tracepoint 0} } {
> +body BpWin::bp_modify {bp_event {tracepoint 0}} {
>    global _bp_en _bp_disp tcl_platform _files
>
> +  array set event [$bp_event get]
>    if {$tracepoint} {
> -    set bpinfo [gdb_get_tracepoint_info $bpnum]
> -    lassign $bpinfo file func line pc enabled pass_count \
> -      step_count thread hit_count actions
> -    set disposition tracepoint
> +    array set event [list disposition tracepoint]
>      set bptype tracepoint
>    } else {
> -    set bpinfo [gdb_get_breakpoint_info $bpnum]
> -    lassign $bpinfo file func line pc type enabled disposition \
> -      ignore_count commands cond thread hit_count
>      set bptype breakpoint
>    }
>
>    set found 0
>    for {set i 1} {$i < $next_row} {incr i} {
> -    if { $bpnum == $index_to_bpnum($i)
> +    if { $event(number) == $index_to_bpnum($i)
>  	 && "$Index_to_bptype($i)" == "$bptype"} {
>        incr found
>        break
> @@ -443,19 +434,19 @@ body BpWin::bp_modify { bpnum {tracepoin
>    }
>
>    if {!$found} {
> -    debug "ERROR: breakpoint number $bpnum not found!"
> +    debug "ERROR: breakpoint number $event(number) not found!"
>      return
>    }
>
> -  if {$_bp_en($i) != $enabled} {
> -    set _bp_en($i) $enabled
> +  if {$_bp_en($i) != $event(enabled)} {
> +    set _bp_en($i) $event(enabled)
>    }
>
> -  if {$_bp_disp($i) != $disposition} {
> -    set _bp_disp($i) $disposition
> +  if {$_bp_disp($i) != $event(disposition)} {
> +    set _bp_disp($i) $event(disposition)
>    }
>
> -  switch $disposition {
> +  switch $event(disposition) {
>      donttouch { set color [pref get gdb/src/bp_fg] }
>      delete {
>        set color [pref get gdb/src/temp_bp_fg]
> @@ -464,7 +455,7 @@ body BpWin::bp_modify { bpnum {tracepoin
>      default { set color yellow}
>    }
>
> -  if {$thread != "-1"} {set color [pref get gdb/src/thread_fg]}
> +  if {$event(thread) != "-1"} {set color [pref get gdb/src/thread_fg]}
>
>    if {$tcl_platform(platform) == "windows"} then {
>      $twin.en$i configure -fg $color
> @@ -472,24 +463,24 @@ body BpWin::bp_modify { bpnum {tracepoin
>      $twin.en$i configure -selectcolor $color
>    }
>    if {$tracepoints} {
> -    $twin.num$i configure  -text "$bpnum "
> +    $twin.num$i configure  -text "$event(number) "
>    }
> -  $twin.addr$i configure -text "$pc "
> -  if {[info exists _files(short,$file)]} {
> -    set file $_files(short,$file)
> +  $twin.addr$i configure -text "$event(address) "
> +  if {[info exists _files(short,$event(file))]} {
> +    array set event [list file $_files(short,$event(file))]
>    } else {
>      # FIXME.  Really need to do better than this.
> -    set file [::file tail $file]
> +    array set event [list file [::file tail $event(file)]]
>    }
>    if {$show_threads} {
> -    if {$thread == "-1"} {set thread "ALL"}
> -    $twin.thread$i configure -text "$thread "
> +    if {$event(thread) == "-1"} {array set event [list thread "ALL"]}
> +    $twin.thread$i configure -text "$event(thread) "
>    }
> -  $twin.file$i configure -text "$file "
> -  $twin.line$i configure  -text "$line "
> -  $twin.func$i configure  -text "$func "
> +  $twin.file$i configure -text "$event(file) "
> +  $twin.line$i configure  -text "$event(line) "
> +  $twin.func$i configure  -text "$event(function) "
>    if {$tracepoints} {
> -    $twin.pass$i configure  -text "$pass_count "
> +    $twin.pass$i configure  -text "$event(pass_count) "
>    }
>  }
>
> @@ -562,9 +553,10 @@ body BpWin::bp_type { i } {
>  # ------------------------------------------------------------------
>  #  METHOD:  bp_delete - delete a breakpoint
>  # ------------------------------------------------------------------
> -body BpWin::bp_delete { bpnum } {
> +body BpWin::bp_delete {bp_event} {
> +  array set event [$bp_event get number]
>    for {set i 1} {$i < $next_row} {incr i} {
> -    if { $bpnum == $index_to_bpnum($i) } {
> +    if { $event(number) == $index_to_bpnum($i) } {
>        if {$tracepoints} {
>  	grid forget $twin.en$i $twin.num$i $twin.addr$i $twin.file$i \
>  	  $twin.line$i $twin.func$i $twin.pass$i
> @@ -587,22 +579,36 @@ body BpWin::bp_delete { bpnum } {
>  }
>
>  # ------------------------------------------------------------------
> -#  METHOD:  update - update widget when a breakpoint changes
> +#  PUBLIC METHOD:  breakpoint - Update widget when a breakpoint
> +#                   event is received from the backend.
>  # ------------------------------------------------------------------
> -body BpWin::update {action bpnum addr {linenum {}} {file {}} {type 0} 
> args} {
> -  #debug "bp update $action $bpnum $type"
> +body BpWin::breakpoint {bp_event} {
>
> -  if {$type == "tracepoint"} {
> -    set tp 1
> -  } else {
> -    set tp 0
> +  array set event [$bp_event get]
> +  #debug "bp update $event(action) $event(number) $event(type)"
> +
> +  switch $event(action) {
> +    modify  { bp_modify $bp_event 0 }
> +    create  { bp_add $bp_event 0 }
> +    delete  { bp_delete $bp_event }
> +    default { dbug E "Unknown breakpoint action: $event(action)" }
>    }
> +}
> +
> +# ------------------------------------------------------------------
> +#  METHOD:  tracepoint - Update widget when a tracepoint event
> +#            is received from the backend.
> +# ------------------------------------------------------------------
> +body BpWin::tracepoint {tp_event} {
> +
> +  array set event [$tp_event get]
> +  #debug "tp update $event(action) $event(number)"
>
> -  switch $action {
> -    modify { bp_modify $bpnum $tp}
> -    create { bp_add $bpnum $tp}
> -    delete { bp_delete $bpnum }
> -    default { debug "Unknown breakpoint action: $action" }
> +  switch $event(action) {
> +    modify  { bp_modify $tp_event 1 }
> +    create  { bp_add $tp_event 1 }
> +    delete  { bp_delete $tp_event }
> +    default { dbug E "Unknown tracepoint action: $event(action)" }
>    }
>  }
>
> Index: srctextwin.ith
> ===================================================================
> RCS file: /cvs/src/src/gdb/gdbtk/library/srctextwin.ith,v
> retrieving revision 1.10
> diff -p -u -r1.10 srctextwin.ith
> --- srctextwin.ith	2001/01/19 16:09:10	1.10
> +++ srctextwin.ith	2001/04/18 22:23:08
> @@ -1,5 +1,5 @@
> -# SrcTextWin class definition, for GDBtk.
> -# Copyright 1997, 1998, 1999 Cygnus Solutions
> +# SrcTextWin class definition, for Insight
> +# Copyright 1997, 1998, 1999, 2001 Red Hat, Inc.
>  #
>  # This program is free software; you can redistribute it and/or modify 
> it
>  # under the terms of the GNU General Public License (GPL) as published 
> by
>
> @@ -13,7 +13,7 @@
>
>
>  class SrcTextWin {
> -  inherit itk::Widget
> +  inherit itk::Widget GDBWin
>
>    public {
>      variable Tracing	;# 1 if we are running in trace mode
>
> %%%%% GDBWin inherits from GDBEventHandler, so SrcTextWin's will get
> events.
>
> @@ -89,6 +89,10 @@ class SrcTextWin {
>      method clear_file {}
>      method get_file {}
>      method set_tag_to_stack {}
> +
> +    # GDB Events
> +    method breakpoint {event}
> +    method tracepoint {event}
>    }
>
>    protected {
> Index: srctextwin.itb
> ===================================================================
> RCS file: /cvs/src/src/gdb/gdbtk/library/srctextwin.itb,v
> retrieving revision 1.22
> diff -p -u -r1.22 srctextwin.itb
> --- srctextwin.itb	2001/03/01 12:30:41	1.22
> +++ srctextwin.itb	2001/04/18 22:23:16
> @@ -1,5 +1,5 @@
> - # Paned text widget for source code, for GDBtk.
> -# Copyright 1997, 1998, 1999 Cygnus Solutions
> +# Paned text widget for source code, for Insight
> +# Copyright 1997, 1998, 1999, 2001 Red Hat, Inc.
>  #
>  # This program is free software; you can redistribute it and/or modify 
> it
>  # under the terms of the GNU General Public License (GPL) as published 
> by
>
> @@ -60,8 +60,6 @@ body SrcTextWin::constructor {args} {
>    build_win
>
>    # add hooks
> -  add_hook gdb_breakpoint_change_hook "$this bp"
> -
>    if {$Tracing} {
>      add_hook control_mode_hook "$this set_control_mode"
>      add_hook gdb_trace_find_hook "$this trace_find_hook"
> @@ -83,7 +81,6 @@ body SrcTextWin::constructor {args} {
>  #  DESTRUCTOR - destroy window containing widget
>  # ------------------------------------------------------------------
>  body SrcTextWin::destructor {} {
> -  remove_hook gdb_breakpoint_change_hook "$this bp"
>    if {$Tracing} {
>      remove_hook control_mode_hook "$this set_control_mode"
>    }
> @@ -1454,6 +1451,27 @@ body SrcTextWin::removeBreakTag {win lin
>  }
>
>  # ------------------------------------------------------------------
> +#  PUBLIC METHOD:  breakpoint - Handle a breakpoint create, delete,
> +#                   or modify event from the backend.
> +# ------------------------------------------------------------------
> +body SrcTextWin::breakpoint {event} {
> +  array set bp_event [$event get]
> +  bp $bp_event(action) $bp_event(number) $bp_event(address) \
> +    $bp_event(line) $bp_event(file) $bp_event(disposition)  \
> +    $bp_event(enabled) $bp_event(thread)
> +}
> +
> +# ------------------------------------------------------------------
> +#  PUBLIC METHOD:  tracepoint - Handle a tracepoint create, delete,
> +#                   modify event from the backend.
> +# ------------------------------------------------------------------
> +body SrcTextWin::tracepoint {event} {
> +  array set tp_event [$event get]
> +  bp $tp_event(action) $tp_event(number) $tp_event(address) \
> +    $tp_event(line) $tp_event(file) tracepoint $tp_event(pass_count)
> +}
> +
> +# ------------------------------------------------------------------
>  #  METHOD:  bp - set and remove breakpoints
>  #
>  #  if $addr is valid, the breakpoint will be set in the assembly or
> @@ -1462,7 +1480,7 @@ body SrcTextWin::removeBreakTag {win lin
>  # ------------------------------------------------------------------
>  body SrcTextWin::bp {action bpnum addr {linenum {}} {file {}} {type 0} 
> {enabled 0}  {thread -1}} {
>  #  debug "$action addr=$addr line=$linenum file=$file type=$type 
> current(filename)=$current(filename)"
> -
> +
>    switch $current(mode) {
>      SOURCE {
>        if {[string compare $file $current(filename)] == 0 && 
> $linenum != {}} {
> @@ -1501,15 +1519,23 @@ body SrcTextWin::bp {action bpnum addr {
>  # ------------------------------------------------------------------
>  body SrcTextWin::do_bp { win action linenum type bpnum enabled thread 
> asm} {
>  #  debug "$action line=$linenum type=$type bpnum=$bpnum 
> enabled=$enabled thread=$thread"
> -
> +
>    if {$dont_change_appearance} {
>      return
>    }
>
> -  if {!$asm && $action == "delete" && [string compare $type 
> tracepoint] != 0} {
> +  if {$action == "delete" && [string compare $type tracepoint] != 0} {
>      # make sure there are no more breakpoints on
>      # this line.
> -    set bps [gdb_find_bp_at_line $current(filename) $linenum]
> +    if {!$asm} {
> +      set bps [gdb_find_bp_at_line $current(filename) $linenum]
> +    } else {
> +      if {[info exists _map($Cname,line=$linenum)]} {
> +	set bps [gdb_find_bp_at_addr $_map($Cname,line=$linenum)]
> +      } else {
> +	set bps {}
> +      }
> +    }
>      if {[llength $bps] > 0} {
>        foreach b $bps {
>  	if {$b != $bpnum} {
>
> %%%%% Ok, this is the bug fix for multiple breakpoints on a single line.
> As you can see, we previously only dealt with SOURCE. Now we do all 
> modes.
>
> @@ -1568,7 +1594,7 @@ body SrcTextWin::do_bp { win action line
>        set tag_type thread_bp_tag
>      }
>      default {
> -      dbug E "UNKNOWN BP TYPE $action $type"
> +      dbug E "UNKNOWN BP TYPE action=\"$action\" type=\"$type\""
>        $win insert $linenum.0 "X" bp_tag
>        set tag_type bp_tag
>      }
> @@ -1903,16 +1929,20 @@ body SrcTextWin::remove_bp_at_line {{win
>    if {$Running} {return}
>
>    # Look up the line...  This foreach is an lassign...
> -
> +
>    foreach {name line addr type} [lookup_line $win $y] {
>      break
>    }
> -
> -  if {[string compare $type src] == 0} {
> -    gdb_cmd "clear $name:$addr"
> -  } else {
> -    gdb_cmd "clear *$addr"
> +
> +  # FIXME: if there are multiple bp/tp at a single line,
> +  # we will (right now) always take the first one we find...
> +  switch $type {
> +    src { set bps [gdb_find_bp_at_line $name $addr] }
> +    asm { set bps [gdb_find_bp_at_addr $addr] }
>    }
> +
> +  set number [lindex $bps 0]
> +  gdb_cmd "delete $number"
>  }
>
>
> %%%%% This was the big (non-obvious) change that I explained earlier.
>
> Index: gdbtk-cmds.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/gdbtk/generic/gdbtk-cmds.c,v
> retrieving revision 1.30
> diff -p -u -r1.30 gdbtk-cmds.c
> --- gdbtk-cmds.c	2001/04/18 17:43:59	1.30
> +++ gdbtk-cmds.c	2001/04/18 22:24:00
> @@ -3766,9 +3766,7 @@ gdb_loadfile (ClientData clientData, Tcl
>   */
>
>  /* This implements the tcl command "gdb_set_bp"
> - * It sets breakpoints, and runs the Tcl command
> - *     gdbtk_tcl_breakpoint create
> - * to register the new breakpoint with the GUI.
> + * It sets breakpoints, and notifies the GUI.
>   *
>   * Tcl Arguments:
>   *    filename: the file in which to set the breakpoint
>
> %%%%% Just updating the comments to reflect reality.
>
> The next few lines deal with the notification. For some reason, we were
> synthesizing our own "event" in gdb_set_bp and gdb_set_bp_addr. I've
> removed all the duplicate code and simply called
> "create_breakpoint_hook". From fifty lines of code to one line. This 
> must
> be a good change. It sure makes much more sense to me. :-)
>
> @@ -3787,7 +3785,7 @@ gdb_set_bp (clientData, interp, objc, ob
>       Tcl_Obj *CONST objv[];
>  {
>    struct symtab_and_line sal;
> -  int line, ret, thread = -1;
> +  int line, thread = -1;
>    struct breakpoint *b;
>    char *buf, *typestr;
>    Tcl_DString cmd;
> @@ -3854,35 +3852,12 @@ gdb_set_bp (clientData, interp, objc, ob
>    free(buf);
>
>    /* now send notification command back to GUI */
> -
> -  Tcl_DStringInit (&cmd);
> -
> -  Tcl_DStringAppend (&cmd, "gdbtk_tcl_breakpoint create ", -1);
> -  xasprintf (&buf, "%d", b->number);
> -  Tcl_DStringAppendElement (&cmd, buf);
> -  free(buf);
> -  xasprintf (&buf, "0x%lx", (long) sal.pc);
> -  Tcl_DStringAppendElement (&cmd, buf);
> -  Tcl_DStringAppendElement (&cmd, Tcl_GetStringFromObj (objv[2], 
> NULL));
> -  Tcl_DStringAppendElement (&cmd, Tcl_GetStringFromObj (objv[1], 
> NULL));
> -  Tcl_DStringAppendElement (&cmd, bpdisp[b->disposition]);
> -  free(buf);
> -  xasprintf (&buf, "%d", b->enable);
> -  Tcl_DStringAppendElement (&cmd, buf);
> -  free(buf);
> -  xasprintf (&buf, "%d", b->thread);
> -  Tcl_DStringAppendElement (&cmd, buf);
> -  free(buf);
> -
> -  ret = Tcl_Eval (interp, Tcl_DStringValue (&cmd));
> -  Tcl_DStringFree (&cmd);
> -  return ret;
> +  create_breakpoint_hook (b);
> +  return TCL_OK;
>  }
>
>  /* This implements the tcl command "gdb_set_bp_addr"
> - * It sets breakpoints, and runs the Tcl command
> - *     gdbtk_tcl_breakpoint create
> - * to register the new breakpoint with the GUI.
> + * It sets breakpoints, and notifies the GUI.
>   *
>   * Tcl Arguments:
>   *    addr: the address at which to set the breakpoint
> @@ -3898,7 +3873,7 @@ gdb_set_bp_addr (ClientData clientData,
>
>  {
>    struct symtab_and_line sal;
> -  int ret, thread = -1;
> +  int thread = -1;
>    long addr;
>    struct breakpoint *b;
>    char *filename, *typestr, *buf;
> @@ -3956,36 +3931,8 @@ gdb_set_bp_addr (ClientData clientData,
>    b->addr_string = xstrdup (buf);
>
>    /* now send notification command back to GUI */
> -
> -  Tcl_DStringInit (&cmd);
> -
> -  Tcl_DStringAppend (&cmd, "gdbtk_tcl_breakpoint create ", -1);
> -  free(buf);
> -  xasprintf (&buf, "%d", b->number);
> -  Tcl_DStringAppendElement (&cmd, buf);
> -  free(buf);
> -  xasprintf (&buf, "0x%lx", addr);
> -  Tcl_DStringAppendElement (&cmd, buf);
> -  free(buf);
> -  xasprintf (&buf, "%d", b->line_number);
> -  Tcl_DStringAppendElement (&cmd, buf);
> -
> -  filename = symtab_to_filename (sal.symtab);
> -  if (filename == NULL)
> -    filename = "";
> -  Tcl_DStringAppendElement (&cmd, filename);
> -  Tcl_DStringAppendElement (&cmd, bpdisp[b->disposition]);
> -  free(buf);
> -  xasprintf (&buf, "%d", b->enable);
> -  Tcl_DStringAppendElement (&cmd, buf);
> -  free(buf);
> -  xasprintf (&buf, "%d", b->thread);
> -  Tcl_DStringAppendElement (&cmd, buf);
> -
> -  ret = Tcl_Eval (interp, Tcl_DStringValue (&cmd));
> -  Tcl_DStringFree (&cmd);
> -  free(buf);
> -  return ret;
> +  create_breakpoint_hook (b);
> +  return TCL_OK;
>  }
>
>  /* This implements the tcl command "gdb_find_bp_at_line"
> @@ -4085,7 +4032,7 @@ gdb_find_bp_at_addr (clientData, interp,
>   * Tcl Result:
>   *   A list with {file, function, line_number, address, type, enabled?,
>   *                disposition, ignore_count, {list_of_commands},
> - *                thread, hit_count}
> + *                condition, thread, hit_count}
>   */
>
>  static int
> Index: gdbtk-hooks.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/gdbtk/generic/gdbtk-hooks.c,v
> retrieving revision 1.9
> diff -p -u -r1.9 gdbtk-hooks.c
> --- gdbtk-hooks.c	2001/04/05 00:04:28	1.9
> +++ gdbtk-hooks.c	2001/04/18 22:24:16
> @@ -661,23 +661,13 @@ breakpoint_notify (b, action)
>       const char *action;
>  {
>    char *buf;
> -  int v;
> -  struct symtab_and_line sal;
> -  char *filename;
>
>    if (b->type != bp_breakpoint)
>      return;
>
>    /* We ensure that ACTION contains no special Tcl characters, so we
>       can do this.  */
> -  sal = find_pc_line (b->address, 0);
> -  filename = symtab_to_filename (sal.symtab);
> -  if (filename == NULL)
> -    filename = "";
> -
> -  xasprintf (&buf, "gdbtk_tcl_breakpoint %s %d 0x%lx %d {%s} {%s} %d 
> %d",
> -	   action, b->number, (long) b->address, b->line_number, filename,
> -	   bpdisp[b->disposition], b->enable, b->thread);
> +  xasprintf (&buf, "gdbtk_tcl_breakpoint %s %d", action, b->number);
>
>    if (Tcl_Eval (gdbtk_interp, buf) != TCL_OK)
>      report_error ();
>
> %%%%% The above (and below) are the changes to the event model. All this
> info is just not needed at this time. (We were calling
> gdb_get_breakpoint_info everytime anyway!)
>
> Note, too, though, that the above code (now removed) contains one HUGE
> bug: xasprintf (&buf, "0x%lx", (long)b->address). Addresses should NEVER
> be printed this way. One should use "paddr_nz" or something to have gdb
> print it, since the host's type sizes could differ from the target's. So
> it should have been: xasprintf (&buf, "0x%s", paddr_nz (b->address)).
>
> @@ -794,19 +784,10 @@ tracepoint_notify (tp, action)
>       const char *action;
>  {
>    char *buf;
> -  int v;
> -  struct symtab_and_line sal;
> -  char *filename;
>
>    /* We ensure that ACTION contains no special Tcl characters, so we
>       can do this.  */
> -  sal = find_pc_line (tp->address, 0);
> -
> -  filename = symtab_to_filename (sal.symtab);
> -  if (filename == NULL)
> -    filename = "N/A";
> -  xasprintf (&buf, "gdbtk_tcl_tracepoint %s %d 0x%lx %d {%s} %d", 
> action, tp->number,
> -	   (long) tp->address, sal.line, filename, tp->pass_count);
> +  xasprintf (&buf, "gdbtk_tcl_tracepoint %s %d", action, tp->number);
>
>    if (Tcl_Eval (gdbtk_interp, buf) != TCL_OK)
>      report_error ();
>
>
>
> %%%%% That's it! Phew! If you got this far, THANKS!
>
> Keith
>
>

_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-_-
Jim Ingham                                                           
jingham@apple.com
Developer Tools - gdb


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]