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

Simon Marchi
2020-10-16T22:45:55Z

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.

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.

> }
> }
>
> @@ -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.

Simon