Re: [PATCHv7 07/11] gdb: parse pending breakpoint thread/task immediately

[Tom Tromey <tom@tromey.com>]

>>>>> "Andrew" == Andrew Burgess <aburgess@redhat.com> writes:

Andrew> As a result of this commit the breakpoints 'extra_string' field is now
Andrew> only used by bp_dprintf type breakpoints to hold the printf format and
Andrew> arguments.  This string should always be empty for other breakpoint
Andrew> types.  This allows me to clean up some code in
Andrew> print_breakpoint_location.

I wonder if this member can be pushed into struct dprintf_breakpoint.
(I don't think you should do that in this series, you've been waiting
long enough.)

Andrew>   2. The handling of '-force-condition' is odd, if this flag appears
Andrew>   immediately after a condition then it will be treated as part of the
Andrew>   condition, e.g.:

We probably should have required this to appear before the linespec.

Maybe also thread/task/etc.

Andrew> +/* Return true if STR starts with PREFIX, otherwise return false.  */
Andrew> +
Andrew> +static bool
Andrew> +startswith (const char *str, const token &prefix)
Andrew> +{
Andrew> +  return strncmp (str, prefix.data (), prefix.length ()) == 0;
Andrew> +}

I wouldn't mind this in common-utils.h alongside the other startswith,
but it's fine here too.

Andrew> +  test ("  if blah  ", "blah");
Andrew> +  test (" if blah thread 1", "blah", global_thread_num);
Andrew> +  test (" if blah inferior 1", "blah", -1, 1);
Andrew> +  test (" if blah thread 1  ", "blah", global_thread_num);

It seems like there are some confounding cases, where gdb would previous
(rightly) give a syntax error, but where now they may be passed through
without notice.

I thought of some... maybe in the "don't care" bucket.  I mean, it would
be nice if we could do a little better, since I guess the user could
typo and then wind up with a breakpoint that's never really useful,
like:

    break some_future_function -force-condition if x == thread 5

Will this ever yield an error?  Maybe the user accidentally left out the
right-hand-side of that 'thread'.  Or maybe meant to write "thread thread".

Probably the worst cases involve deliberately confounding macro
definitions, but these IMO are definitely "don't care".  Maybe advising
the user to surround the 'if' in parens and that's enough, assuming:

    break func if ( x == thread )

... does the right thing.

Tom