This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [patch] Add visible flag to breakpoints.


The non-python bits are fine, pending a couple of small issues
I neglected mentioning before.  Sorry about that.

> +
> +/* Set a breakpoint.  This function is shared between CLI and MI
> +   functions for setting a breakpoint.  It wraps create_new_breakpoint
> +   and never asks for an internal breakpoint number to be allocated
> +   against the breakpoint.  Returns true if any breakpoint was
> +   created; false otherwise.  */
> +
> +int
> +create_breakpoint (struct gdbarch *gdbarch,
> +                  char *arg, 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)
> +{
> +  return create_new_breakpoint (gdbarch, arg, cond_string, thread,
> +                               parse_condition_and_thread, tempflag,
> +                               type_wanted, ignore_count,
> +                               pending_break_support,
> +                               ops, from_tty, enabled, 0);
> +}

Having both create_breakpoint and create_new_breakpoint exported
in breakpoint.h is quite reduntant API-wise, and by just looking at
the .h even confusing, given that "_new_" does not describe
anything that create_breakpoint does not do as well (create_breakpoint
creates a breakpoint, so of course it also creates a _new_ breakpoint!).

Please just add the new flag to create_breakpoint, and do the
trivial update to all its callers.  There are only 3 outside
breakpoint.c, I think.

> -         set_breakpoint_count (breakpoint_count + 1);
> -         b->number = breakpoint_count;
> +         if (internal)
> +           b->number = internal_breakpoint_number--;
> +         else
> +           {
> +             set_breakpoint_count (breakpoint_count + 1);
> +             b->number = breakpoint_count;
> +           }

It would be nice if this pattern that now appears several times
in breakpoint.c would be abstrated out into a small helper function.

-- 
Pedro Alves


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