[PATCH 1/1] gdb, python: fix python breakpoint with extra spec

[Keith Seitz]

On 6/2/23 03:59, Christina Schimpe via Gdb-patches wrote:
> For example this works as expected:
> 
> (gdb) python bp1 = Breakpoint ("file:42")
> 
> But here "thread 1000" is silently ignored:
> 
> (gdb) python bp1 = Breakpoint ("file:42 thread 1000")

This doesn't sound particularly idiomatic/python-y.

Gdb already has getters/setters for thread, task, and other
related properties. Are those insufficient in some way?

The documentation for gdb.Breakpoint.__init__ says:

"Create a new breakpoint according to spec, which is a string
naming the location of a breakpoint."

While it happens to work, "thread 1000" is not part of the
location. That create_breakpoint() accepts this is simply a
(messy) implementation detail that has been convenient for the
CLI interpreter.

If it is desired to be able to set breakpoint properties during
construction, is expanding the ctor to accept a keyword not a
viable, if not cleaner, option?

Keith

> This patch modifies `create_breakpoint` function call from the Python
> implementation so that full spec string is processed.  Unnecessary
> string duplication for "spec" is removed as it is not used after the
> call (tests still pass).
> ---
>   gdb/python/py-breakpoint.c                 | 12 +++++-------
>   gdb/testsuite/gdb.python/py-breakpoint.exp |  4 +++-
>   2 files changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/gdb/python/py-breakpoint.c b/gdb/python/py-breakpoint.c
> index d11fc64df20..3a2f8f5f2b8 100644
> --- a/gdb/python/py-breakpoint.c
> +++ b/gdb/python/py-breakpoint.c
> @@ -893,6 +893,8 @@ bppy_init (PyObject *self, PyObject *args, PyObject *kwargs)
>     bppy_pending_object->number = -1;
>     bppy_pending_object->bp = NULL;
>   
> +  spec = skip_spaces (spec);
> +
>     try
>       {
>         switch (type)
> @@ -908,11 +910,7 @@ bppy_init (PyObject *self, PyObject *args, PyObject *kwargs)
>   
>   	    if (spec != NULL)
>   	      {
> -		gdb::unique_xmalloc_ptr<char>
> -		  copy_holder (xstrdup (skip_spaces (spec)));
> -		const char *copy = copy_holder.get ();
> -
> -		locspec  = string_to_location_spec (&copy,
> +		locspec  = string_to_location_spec (&spec,
>   						    current_language,
>   						    func_name_match_type);
>   	      }
> @@ -941,8 +939,8 @@ bppy_init (PyObject *self, PyObject *args, PyObject *kwargs)
>   	      = breakpoint_ops_for_location_spec (locspec.get (), false);
>   
>   	    create_breakpoint (gdbpy_enter::get_gdbarch (),
> -			       locspec.get (), NULL, -1, NULL, false,
> -			       0,
> +			       locspec.get (), NULL, -1, spec, false,
> +			       1,
>   			       temporary_bp, type,
>   			       0,
>   			       AUTO_BOOLEAN_TRUE,
> diff --git a/gdb/testsuite/gdb.python/py-breakpoint.exp b/gdb/testsuite/gdb.python/py-breakpoint.exp
> index 76094c95d10..3c747677fc7 100644
> --- a/gdb/testsuite/gdb.python/py-breakpoint.exp
> +++ b/gdb/testsuite/gdb.python/py-breakpoint.exp
> @@ -170,8 +170,10 @@ proc_with_prefix test_bkpt_cond_and_cmds { } {
>   
>       # Test conditional setting.
>       set bp_location1 [gdb_get_line_number "Break at multiply."]
> -    gdb_py_test_silent_cmd "python bp1 = gdb.Breakpoint (\"$bp_location1\")" \
> +    gdb_py_test_silent_cmd "python bp1 = gdb.Breakpoint (\"$bp_location1 thread 1\")" \
>   	"Set breakpoint" 0
> +    gdb_test "python print (bp1.thread == 1)" "True" \
> +	"Extra thread spec has been parsed"
>       gdb_continue_to_breakpoint "Break at multiply" \
>   	".*Break at multiply.*"
>       gdb_py_test_silent_cmd "python bp1.condition = \"i == 5\"" \