RE: [PATCH v4 2/2] gdb/breakpoint: add flags to 'condition' and 'break' commands to force condition

Aktemur, Tankut Baris via Gdb-patches
2020-10-19T13:59:12Z

On Saturday, October 17, 2020 12:46 AM, Simon Marchi wrote:
> I don't have time to review this in as much depth as I'd like, but I
> took a quick look at the code and tested the user experience, and it
> looks good to me. If nobody else has anything to add in ~1 week from
> now, I'm fine with you merging this.

Hi Simon,

Thank you for your comments. I'll wait for ~1 week as you suggest, and then merge
both patches.

> Some small comments below:
>
> On 2020-10-13 8:25 a.m., Tankut Baris Aktemur via Gdb-patches wrote:
> > @@ -950,9 +950,11 @@ set_breakpoint_condition (struct breakpoint *b, const char *exp,
> > catch (const gdb_exception_error &e)
> > {
> > /* Condition string is invalid. If this happens to
> > - be the last loc, abandon. */
> > + be the last loc, abandon (if not forced) or continue
> > + (if forced). */
> > if (loc->next == nullptr)
> > - throw;
> > + if (!force)
> > + throw;
>
> Either do
>
> if (loc->next == nullptr && !force)
> throw;
>
> ... or add braces to the outer if.

Fixed by merging the conditions with a '&&'.

>
> > }
> > }
> >
> > @@ -1032,6 +1034,18 @@ condition_command (const char *arg, int from_tty)
> > error_no_arg (_("breakpoint number"));
> >
> > p = arg;
> > +
> > + /* Check if the "-force" flag was passed. */
> > + bool force = false;
> > + const char *tok = skip_spaces (p);
> > + const char *end_tok = skip_to_space (tok);
> > + int toklen = end_tok - tok;
> > + if (toklen >= 1 && strncmp (tok, "-force", toklen) == 0)
> > + {
> > + force = true;
> > + p = end_tok + 1;
> > + }
> > +
>
> Is there a chance you could use the gdb::option framework here? That
> should give tab-completion for free, in addition to simplify the
> parsing. I've never used it myself, so I don't know for sure it applies
> here, but it's worth checking.

I checked it and yes, it's applicable. I found other similar uses (like the '-gid'
flag for 'info threads'). But let me send this as a separate, follow-up patch, to
make the review process easier. OK with that?

-Baris

Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Gary Kershaw
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928