[PATCH] Inline build_bp_list into its only caller

[PATCH] Inline build_bp_list into its only caller

[Tom Tromey]

While working on safety patches for py-breakpoint.c, I ran across
build_bp_list.  This is called from just one place, and it seems
cleaner to inline it into its sole caller.
---
 gdb/python/py-breakpoint.c | 35 +++++++++++++----------------------
 1 file changed, 13 insertions(+), 22 deletions(-)

diff --git a/gdb/python/py-breakpoint.c b/gdb/python/py-breakpoint.c
index ecb42cee5f9..e5df4459dd6 100644
--- a/gdb/python/py-breakpoint.c
+++ b/gdb/python/py-breakpoint.c
@@ -1089,26 +1089,6 @@ bppy_repr (PyObject *self)
 			       bp->bp->hit_count, str.c_str ());
 }
 
-/* Append to LIST the breakpoint Python object associated to B.
-
-   Return true on success.  Return false on failure, with the Python error
-   indicator set.  */
-
-static bool
-build_bp_list (struct breakpoint *b, PyObject *list)
-{
-  PyObject *bp = (PyObject *) b->py_bp_object;
-
-  /* Not all breakpoints will have a companion Python object.
-     Only breakpoints that were created via bppy_new, or
-     breakpoints that were created externally and are tracked by
-     the Python Scripting API.  */
-  if (bp == nullptr)
-    return true;
-
-  return PyList_Append (list, bp) == 0;
-}
-
 /* See python-internal.h.  */
 
 bool
@@ -1144,8 +1124,19 @@ gdbpy_breakpoints (PyObject *self, PyObject *args)
   /* If build_bp_list returns false, it signals an error condition.  In that
      case abandon building the list and return nullptr.  */
   for (breakpoint &bp : all_breakpoints ())
-    if (!build_bp_list (&bp, list.get ()))
-      return nullptr;
+    {
+      PyObject *bp_obj = (PyObject *) bp.py_bp_object;
+
+      /* Not all breakpoints will have a companion Python object.
+	 Only breakpoints that were created via bppy_new, or
+	 breakpoints that were created externally and are tracked by
+	 the Python Scripting API.  */
+      if (bp_obj == nullptr)
+	continue;
+
+      if (PyList_Append (list.get (), bp_obj) < 0)
+	return nullptr;
+    }
 
   return PyList_AsTuple (list.get ());
 }
-- 
2.49.0

Re: [PATCH] Inline build_bp_list into its only caller

[Simon Marchi]

On 9/13/26 1:35 PM, Tom Tromey wrote:
> While working on safety patches for py-breakpoint.c, I ran across
> build_bp_list.  This is called from just one place, and it seems
> cleaner to inline it into its sole caller.
> ---
>  gdb/python/py-breakpoint.c | 35 +++++++++++++----------------------
>  1 file changed, 13 insertions(+), 22 deletions(-)
> 
> diff --git a/gdb/python/py-breakpoint.c b/gdb/python/py-breakpoint.c
> index ecb42cee5f9..e5df4459dd6 100644
> --- a/gdb/python/py-breakpoint.c
> +++ b/gdb/python/py-breakpoint.c
> @@ -1089,26 +1089,6 @@ bppy_repr (PyObject *self)
>  			       bp->bp->hit_count, str.c_str ());
>  }
>  
> -/* Append to LIST the breakpoint Python object associated to B.
> -
> -   Return true on success.  Return false on failure, with the Python error
> -   indicator set.  */
> -
> -static bool
> -build_bp_list (struct breakpoint *b, PyObject *list)
> -{
> -  PyObject *bp = (PyObject *) b->py_bp_object;


> -
> -  /* Not all breakpoints will have a companion Python object.
> -     Only breakpoints that were created via bppy_new, or
> -     breakpoints that were created externally and are tracked by
> -     the Python Scripting API.  */
> -  if (bp == nullptr)
> -    return true;
> -
> -  return PyList_Append (list, bp) == 0;
> -}
> -
>  /* See python-internal.h.  */
>  
>  bool
> @@ -1144,8 +1124,19 @@ gdbpy_breakpoints (PyObject *self, PyObject *args)
>    /* If build_bp_list returns false, it signals an error condition.  In that

This comment refers to the new removed function.

>       case abandon building the list and return nullptr.  */
>    for (breakpoint &bp : all_breakpoints ())
> -    if (!build_bp_list (&bp, list.get ()))
> -      return nullptr;
> +    {
> +      PyObject *bp_obj = (PyObject *) bp.py_bp_object;

I'd remove the `bp` variable and use bp.py_bp_object directly.

Otherwise, LGTM.

Approved-By: Simon Marchi <simon.marchi@efficios.com>

Simon