This is the mail archive of the
mailing list for the GDB project.
Re: [patch] Add visible flag to breakpoints.
- From: Pedro Alves <pedro at codesourcery dot com>
- To: gdb-patches at sourceware dot org, pmuldoon at redhat dot com
- Cc: Tom Tromey <tromey at redhat dot com>, dan at codesourcery dot com
- Date: Sun, 31 Oct 2010 21:07:37 +0000
- Subject: Re: [patch] Add visible flag to breakpoints.
- References: <email@example.com> <firstname.lastname@example.org> <email@example.com>
On Friday 22 October 2010 22:05:30, Phil Muldoon wrote:
> -/* Set a breakpoint. This function is shared between CLI and MI
> +/* Set a breakpoint. This function is shared between CLI and MI
Spurious, and incorrect whitespace change.
> functions for setting a breakpoint. This function has two major
> modes of operations, selected by the PARSE_CONDITION_AND_THREAD
> parameter. If non-zero, the function will parse arg, extracting
> breakpoint location, address and thread. Otherwise, ARG is just the
> location of breakpoint, with condition and thread specified by the
> - COND_STRING and THREAD parameters. Returns true if any breakpoint
> - was created; false otherwise. */
> + COND_STRING and THREAD parameters. If INTERNAL is non-zero, the
> + breakpoint number will be allocated from the internal breakpoint
> + count. Returns true if any breakpoint was created; false
> + otherwise. */
Two spaces after period, not three. Don't lose the empty line
between comment and function.
> create_breakpoint (struct gdbarch *gdbarch,
> @@ -7750,7 +7785,8 @@ break_command_1 (char *arg, int flag, int from_tty)
> NULL /* breakpoint_ops */,
> - 1 /* enabled */);
> + 1 /* enabled */,
> + 0 /* internal */);
Inconsistent whitespace with the other cases. Please fix that
in all the several places this appears.
> int static_trace_marker_id_idx;
> - };
> + /* With a Python scripting enabled GDB, store a reference to the
> + Python object that has been associated with this breakpoint.
> + This is always NULL for a GDB that is not script enabled. It
> + can sometimes be NULL for enabled GDBs as not all breakpoint
> + types are tracked by the Python scripting API. */
> + PyObject *py_bp_object;
Is the indentation correct here? Looks two spaces too much to the right.
> + if (internal)
> + /* Do not mention breakpoints with a negative number, but do
> + notify observers. */
> + observer_notify_breakpoint_created (b->number);
> + else
> + mention (b);
Same here. Please double check your patch for formatting -- there may be
other cases I missed.
> extern int create_breakpoint (struct gdbarch *gdbarch, char *arg,
> - char *cond_string, int thread,
> - int parse_condition_and_thread,
> - int tempflag, enum bptype wanted_type,
> - int ignore_count,
> - enum auto_boolean pending_break_support,
> - struct breakpoint_ops *ops,
> - int from_tty,
> - int enabled);
> + char *cond_string, int thread,
> + int parse_condition_and_thread,
> + int tempflag,
> + enum bptype type_wanted,
> + int ignore_count,
> + enum auto_boolean pending_break_support,
> + struct breakpoint_ops *ops,
> + int from_tty,
> + int enabled,
> + int internal);
Leftover reindent from "create_new_breakpoint"? Doesn't
look necessary or correct? Please undo.
> create_breakpoint (python_gdbarch,
> - spec, NULL, -1,
> - 0,
> - 0, bp_breakpoint,
> - 0,
> - AUTO_BOOLEAN_TRUE,
> - NULL, 0, 1);
> + spec, NULL, -1,
> + 0,
> + 0, bp_breakpoint,
> + 0,
> + AUTO_BOOLEAN_TRUE,
> + NULL, 0, 1, internal_bp);
Otherwise looks fine to me. Thanks.