Re: [patch] Add visible flag to breakpoints.

[Pedro Alves <pedro@codesourcery.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.  */
>  int

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)
>                      pending_break_support,
>                      NULL /* breakpoint_ops */,
>                      from_tty,
> -                    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);

Same here?

Otherwise looks fine to me.  Thanks.

-- 
Pedro Alves