Addressing review comments on Callback field, using BreakpointExistsError

Author: archanaravindar

pkg/proc/breakpoints.go

@@ -108,23 +108,22 @@ type Breaklet struct {
 	// the return value will determine if the breaklet should be considered
 	// active.
 	// The callback can have side-effects.
-	Callback func(th Thread, p *Target) (bool, error)
+	callback func(th Thread, p *Target) (bool, error)
 
 	// For WatchOutOfScopeBreakpoints and StackResizeBreakpoints the watchpoint
 	// field contains the watchpoint related to this out of scope sentinel.
 	watchpoint *Breakpoint
-
 }
 
 // SetCallBack sets the call back field, this was primarily added to prevent exporting callback field
 func (B *Breaklet) SetCallBack(callback DynCallFn) {
-	B.Callback = callback
+	B.callback = callback
 }
+
 // BreakpointKind determines the behavior of delve when the
 // breakpoint is reached.
 type BreakpointKind uint16
 
-
 const (
 	// UserBreakpoint is a user set breakpoint
 	UserBreakpoint BreakpointKind = (1 << iota)
@@ -347,9 +346,9 @@ func (bpstate *BreakpointState) checkCond(tgt *Target, breaklet *Breaklet, threa
 	}
 
 	if active {
-		if breaklet.Callback != nil {
+		if breaklet.callback != nil {
 			var err error
-			active, err = breaklet.Callback(thread, tgt)
+			active, err = breaklet.callback(thread, tgt)
 			if err != nil && bpstate.CondError == nil {
 				bpstate.CondError = err
 			}

pkg/proc/stackwatch.go

@@ -60,7 +60,7 @@ func (t *Target) setStackWatchBreakpoints(scope *EvalScope, watchpoint *Breakpoi
 		deferbreaklet := deferbp.Breaklets[len(deferbp.Breaklets)-1]
 		deferbreaklet.checkPanicCall = true
 		deferbreaklet.watchpoint = watchpoint
-		deferbreaklet.Callback = woos
+		deferbreaklet.callback = woos
 	}
 
 	retbp, err := t.SetBreakpoint(0, retframe.Current.PC, WatchOutOfScopeBreakpoint, retFrameCond)
@@ -70,7 +70,7 @@ func (t *Target) setStackWatchBreakpoints(scope *EvalScope, watchpoint *Breakpoi
 
 	retbreaklet := retbp.Breaklets[len(retbp.Breaklets)-1]
 	retbreaklet.watchpoint = watchpoint
-	retbreaklet.Callback = woos
+	retbreaklet.callback = woos
 
 	if recorded, _ := t.recman.Recorded(); recorded && retframe.Current.Fn != nil {
 		// Must also set a breakpoint on the call instruction immediately
@@ -88,7 +88,7 @@ func (t *Target) setStackWatchBreakpoints(scope *EvalScope, watchpoint *Breakpoi
 				}
 				retbreaklet2 := retbp2.Breaklets[len(retbp.Breaklets)-1]
 				retbreaklet2.watchpoint = watchpoint
-				retbreaklet2.Callback = woos
+				retbreaklet2.callback = woos
 				break
 			}
 		}
@@ -111,7 +111,7 @@ func (t *Target) setStackWatchBreakpoints(scope *EvalScope, watchpoint *Breakpoi
 
 	rszbreaklet := rszbp.Breaklets[len(rszbp.Breaklets)-1]
 	rszbreaklet.watchpoint = watchpoint
-	rszbreaklet.Callback = func(th Thread, _ *Target) (bool, error) {
+	rszbreaklet.callback = func(th Thread, _ *Target) (bool, error) {
 		adjustStackWatchpoint(t, th, watchpoint)
 		return false, nil // we never want this breakpoint to be shown to the user
 	}

pkg/proc/target.go

@@ -403,7 +403,7 @@ func (t *Target) createPluginOpenBreakpoint() {
 		if err != nil {
 			t.BinInfo().logger.Errorf("could not set plugin.Open breakpoint: %v", err)
 		} else {
-			bp.Breaklets[len(bp.Breaklets)-1].Callback = t.pluginOpenCallback
+			bp.Breaklets[len(bp.Breaklets)-1].callback = t.pluginOpenCallback
 		}
 	}
 }

pkg/proc/target_exec.go

@@ -906,7 +906,7 @@ func next(dbp *Target, stepInto, inlinedStepOut bool) error {
 			bp, err := dbp.SetBreakpoint(0, rangeFrames[1].Call.PC, NextBreakpoint, astutil.And(sameGCond, frameoffCondition(&rangeFrames[1])))
 			if err == nil {
 				bplet := bp.Breaklets[len(bp.Breaklets)-1]
-				bplet.Callback = func(th Thread, p *Target) (bool, error) {
+				bplet.callback = func(th Thread, p *Target) (bool, error) {
 					rangeFrameInactivateNextBreakpoints(p, rangeFrames[0].Call.Fn)
 					return false, nil
 				}
@@ -964,7 +964,7 @@ func setStepIntoBreakpoints(dbp *Target, curfn *Function, text []AsmInstruction,
 				return err
 			}
 			breaklet := bp.Breaklets[len(bp.Breaklets)-1]
-			breaklet.Callback = stepIntoCallback
+			breaklet.callback = stepIntoCallback
 		}
 	}
 	if gostmt {
@@ -1246,7 +1246,7 @@ func setStepIntoNewProcBreakpoint(p *Target, sameGCond ast.Expr) {
 		return
 	}
 	blet := bp.Breaklets[len(bp.Breaklets)-1]
-	blet.Callback = func(th Thread, p *Target) (bool, error) {
+	blet.callback = func(th Thread, p *Target) (bool, error) {
 		// Clear temp breakpoints that exist and set a new one for goroutine
 		// newg.goid on the go statement's target
 		scope, err := ThreadScope(p, th)

service/debugger/debugger.go

@@ -1488,7 +1488,7 @@ func (d *Debugger) traverse(t proc.ValidTargets, f *proc.Function, depth int, fo
 					if err != nil {
 						return false, fmt.Errorf("failed to disassemble instruction: %w", err)
 					}
-					
+
 					// Extract address from the decoded instruction's destination location
 					var addr uint64
 					if len(disasm) > 0 && disasm[0].DestLoc != nil {
@@ -1516,9 +1516,9 @@ func (d *Debugger) traverse(t proc.ValidTargets, f *proc.Function, depth int, fo
 					}
 					return false, nil
 				}
-		for _, dynbrklet_i := range dynbp.Breaklets {
-				dynbrklet_i.SetCallBack(dynCallback)
-			}
+				for _, dynbrklet_i := range dynbp.Breaklets {
+					dynbrklet_i.SetCallBack(dynCallback)
+				}
 			}
 
 			if instr.IsCall() && instr.DestLoc != nil && instr.DestLoc.Fn != nil {
@@ -1545,9 +1545,9 @@ func createFnTracepoint(d *Debugger, fname string, rootstr string, followCalls i
 
 	tbp, err1 := d.createBreakpointInternal(&api.Breakpoint{FunctionName: fname, Tracepoint: true, RootFuncName: rootstr, Stacktrace: 20, TraceFollowCalls: followCalls}, "", nil, false)
 	if tbp == nil {
-		if err1 != nil && strings.Contains(err1.Error(), "Breakpoint exists") {
+		if _, exists := err1.(proc.BreakpointExistsError); exists {// ;err1 != nil && strings.Contains(err1.Error(), "Breakpoint exists") {
 			// This is expected
-		} else {
+		} else if err1 != nil {
 			return nil, fmt.Errorf("error creating breakpoint at function %s", fname)
 		}
 	}
@@ -1556,7 +1556,7 @@ func createFnTracepoint(d *Debugger, fname string, rootstr string, followCalls i
 	for i := range raddrs {
 		rtbp, err := d.createBreakpointInternal(&api.Breakpoint{Addr: raddrs[i], TraceReturn: true, RootFuncName: rootstr, Stacktrace: 20, TraceFollowCalls: followCalls}, "", nil, false)
 		if rtbp == nil {
-			if err != nil && strings.Contains(err.Error(), "Breakpoint exists") {
+			if _, exists := err.(proc.BreakpointExistsError); exists { // != nil && strings.Contains(err.Error(), "Breakpoint exists") {
 				// This is expected
 			} else {
 				return nil, fmt.Errorf("error creating breakpoint at function return %s", fname)