diff mbox series

[2/3] Fix latent bug in Python breakpoint creation

Message ID 20221208191804.3819129-3-tromey@adacore.com
State New
Headers
Series Add "announce" flag to Python breakpoint creation |

Commit Message

Tom Tromey Dec. 8, 2022, 7:18 p.m. UTC 
  While working on the previous patch, I noticed that Python breakpoint
creation does:

-	  = (qualified != NULL && PyObject_IsTrue (qualified)

PyObject_IsTrue can fail, so this is missing an error check.  This
patch adds the missing check.

Note that this could probably be improved by using the "p" format in
the call to gdb_PyArg_ParseTupleAndKeywords, but that was added in
Python 3.3, and I think gdb still supports 3.2.
---
 gdb/python/py-breakpoint.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

Comments

Simon Marchi Jan. 12, 2023, 5:44 p.m. UTC | #1 
On 12/8/22 14:18, Tom Tromey via Gdb-patches wrote:
> While working on the previous patch, I noticed that Python breakpoint
> creation does:
> 
> -	  = (qualified != NULL && PyObject_IsTrue (qualified)
> 
> PyObject_IsTrue can fail, so this is missing an error check.  This
> patch adds the missing check.
> 
> Note that this could probably be improved by using the "p" format in
> the call to gdb_PyArg_ParseTupleAndKeywords, but that was added in
> Python 3.3, and I think gdb still supports 3.2.
> ---
>  gdb/python/py-breakpoint.c | 16 ++++++++++++----
>  1 file changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/gdb/python/py-breakpoint.c b/gdb/python/py-breakpoint.c
> index 917fd367d06..39d9bd5dff6 100644
> --- a/gdb/python/py-breakpoint.c
> +++ b/gdb/python/py-breakpoint.c
> @@ -818,7 +818,7 @@ bppy_init_validate_args (const char *spec, char *source,
>  static void
>  bppy_create_breakpoint (enum bptype type, int access_type, int temporary_bp,
>  			int internal_bp, const char *spec,
> -			PyObject *qualified, const char *source,
> +			int qualified, const char *source,

You might as well make this parameter "bool".

Otherwise, LGTM.

Simon
diff mbox series

Patch

diff --git a/gdb/python/py-breakpoint.c b/gdb/python/py-breakpoint.c
index 917fd367d06..39d9bd5dff6 100644
--- a/gdb/python/py-breakpoint.c
+++ b/gdb/python/py-breakpoint.c
@@ -818,7 +818,7 @@  bppy_init_validate_args (const char *spec, char *source,
 static void
 bppy_create_breakpoint (enum bptype type, int access_type, int temporary_bp,
 			int internal_bp, const char *spec,
-			PyObject *qualified, const char *source,
+			int qualified, const char *source,
 			const char *function, const char *label,
 			const char *line)
 {
@@ -829,7 +829,7 @@  bppy_create_breakpoint (enum bptype type, int access_type, int temporary_bp,
       {
 	location_spec_up locspec;
 	symbol_name_match_type func_name_match_type
-	  = (qualified != NULL && PyObject_IsTrue (qualified)
+	  = (qualified
 	     ? symbol_name_match_type::FULL
 	     : symbol_name_match_type::WILD);
 
@@ -916,14 +916,15 @@  bppy_init (PyObject *self, PyObject *args, PyObject *kwargs)
   char *label = NULL;
   char *source = NULL;
   char *function = NULL;
-  PyObject * qualified = NULL;
+  PyObject *qualified_obj = nullptr;
+  int qualified = 0;
 
   if (!gdb_PyArg_ParseTupleAndKeywords (args, kwargs, "|siiOOsssOO", keywords,
 					&spec, &type, &access_type,
 					&internal,
 					&temporary, &source,
 					&function, &label, &lineobj,
-					&qualified))
+					&qualified_obj))
     return -1;
 
 
@@ -955,6 +956,13 @@  bppy_init (PyObject *self, PyObject *args, PyObject *kwargs)
 	return -1;
     }
 
+  if (qualified_obj != nullptr)
+    {
+      qualified = PyObject_IsTrue (qualified_obj);
+      if (qualified == -1)
+	return -1;
+    }
+
   if (bppy_init_validate_args (spec, source, function, label, line.get (),
 			       type) == -1)
     return -1;