Re: [PATCHv9 01/14] gdb: create_breakpoint: add asserts and additional comments

Andrew Burgess
2024-03-31T10:27:47Z

Andrew Burgess <aburgess@redhat.com> writes:

> This commit extends the asserts on create_breakpoint (in the header
> file), and adds some additional assertions into the definition.
>
> The new assert confirms that when the thread and inferior information
> is going to be parsed from the extra_string, then the thread and
> inferior arguments should be -1. That is, the caller of
> create_breakpoint should not try to create a thread/inferior specific
> breakpoint by *both* specifying thread/inferior *and* asking to parse
> the extra_string, it's one or the other.
>
> There should be no user visible changes after this commit.

I've gone ahead and committed this patch.

Thanks,
Andrew

> ---
> gdb/breakpoint.c | 6 ++++++
> gdb/breakpoint.h | 16 ++++++++++++++++
> 2 files changed, 22 insertions(+)
>
> diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
> index 102bd7fad41..7ade82663f9 100644
> --- a/gdb/breakpoint.c
> +++ b/gdb/breakpoint.c
> @@ -9219,6 +9219,12 @@ create_breakpoint (struct gdbarch *gdbarch,
> gdb_assert (inferior == -1 || inferior > 0);
> gdb_assert (thread == -1 || inferior == -1);
>
> + /* If PARSE_EXTRA is true then the thread and inferior details will be
> + parsed from the EXTRA_STRING, the THREAD and INFERIOR arguments
> + should be -1. */
> + gdb_assert (!parse_extra || thread == -1);
> + gdb_assert (!parse_extra || inferior == -1);
> +
> gdb_assert (ops != NULL);
>
> /* If extra_string isn't useful, set it to NULL. */
> diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
> index 226e4d06993..2e2fe1d32e5 100644
> --- a/gdb/breakpoint.h
> +++ b/gdb/breakpoint.h
> @@ -1610,6 +1610,22 @@ enum breakpoint_create_flags
> the FORCE_CONDITION parameter is ignored and the corresponding argument
> is parsed from EXTRA_STRING.
>
> + The THREAD should be a global thread number, the created breakpoint will
> + only apply for that thread. If the breakpoint should apply for all
> + threads then pass -1. However, if PARSE_EXTRA is non-zero then the
> + THREAD parameter is ignored and an optional thread number will be parsed
> + from EXTRA_STRING.
> +
> + The INFERIOR should be a global inferior number, the created breakpoint
> + will only apply for that inferior. If the breakpoint should apply for
> + all inferiors then pass -1. However, if PARSE_EXTRA is non-zero then
> + the INFERIOR parameter is ignored and an optional inferior number will
> + be parsed from EXTRA_STRING.
> +
> + At most one of THREAD and INFERIOR should be set to a value other than
> + -1; breakpoints can be thread specific, or inferior specific, but not
> + both.
> +
> If INTERNAL is non-zero, the breakpoint number will be allocated
> from the internal breakpoint count.
>
> --
> 2.25.4