diff mbox

[RFA,23/23] Use gdb_argv_up in Python

Message ID 20170503224626.2818-24-tom@tromey.com
State New, archived
Headers

Commit Message

Tom Tromey May 3, 2017, 10:46 p.m. UTC 
  This changes one spot in the Python code to use gdb_argv_up.  This
removes the last cleanup from the Python layer.

2017-05-02  Tom Tromey  <tom@tromey.com>

	* python/py-param.c (compute_enum_values): Use gdb_argv_up.
---
 gdb/ChangeLog         |  4 ++++
 gdb/python/py-param.c | 21 +++++----------------
 2 files changed, 9 insertions(+), 16 deletions(-)

Comments

Pedro Alves June 5, 2017, 4:29 p.m. UTC | #1 
On 05/03/2017 11:46 PM, Tom Tromey wrote:
> This changes one spot in the Python code to use gdb_argv_up.  This
> removes the last cleanup from the Python layer.

Hurray!  Thanks so much for all this work.

> -      self->enumeration[i]
> -	= python_string_to_host_string (item.get ()).release ();
> +      enumeration[i] = python_string_to_host_string (item.get ()).release ();
>        if (self->enumeration[i] == NULL)

I think you need to adjust the if too:

       if (enumeration[i] == NULL)


> -	{
> -	  do_cleanups (back_to);
> -	  return 0;
> -	}
> -      make_cleanup (xfree, (char *) self->enumeration[i]);
> +	return 0;
>      }
>  
> -  discard_cleanups (back_to);
> +  self->enumeration = const_cast<const char**> (enumeration.release ());
>    return 1;

If I'm reading the code correctly, "self->enumeration" is never 
released (by design), right?

Thanks,
Pedro Alves
Tom Tromey July 19, 2017, 10:26 p.m. UTC | #2 
>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

>> if (self->enumeration[i] == NULL)

Pedro> I think you need to adjust the if too:
Pedro>        if (enumeration[i] == NULL)

Thanks, I made this fix.

>> +  self->enumeration = const_cast<const char**> (enumeration.release ());

Pedro> If I'm reading the code correctly, "self->enumeration" is never 
Pedro> released (by design), right?

Yes.  I think it could be destroyed if the command is deleted, but I
can't remember now if that can happen; and anyway if it can it must be a
weird thing to do.

Tom
Tom Tromey July 19, 2017, 10:29 p.m. UTC | #3 
>>>>> "Tom" == Tom Tromey <tom@tromey.com> writes:

Tom> Yes.  I think it could be destroyed if the command is deleted, but I
Tom> can't remember now if that can happen; and anyway if it can it must be a
Tom> weird thing to do.

... what I mean here is that it currently is never freed, but I think
(not sure) it could be freed in theory when the command is destroyed.

Tom
diff mbox

Patch

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 4dc3bb4..12b9755 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,9 @@ 
 2017-05-02  Tom Tromey  <tom@tromey.com>
 
+	* python/py-param.c (compute_enum_values): Use gdb_argv_up.
+
+2017-05-02  Tom Tromey  <tom@tromey.com>
+
 	* utils.h (struct gdb_argv_deleter): New.
 	(gdb_argv_up): New typedef.
 	(gdb_buildargv): Change return type.
diff --git a/gdb/python/py-param.c b/gdb/python/py-param.c
index f0d3423..0afed32 100644
--- a/gdb/python/py-param.c
+++ b/gdb/python/py-param.c
@@ -555,7 +555,6 @@  static int
 compute_enum_values (parmpy_object *self, PyObject *enum_values)
 {
   Py_ssize_t size, i;
-  struct cleanup *back_to;
 
   if (! enum_values)
     {
@@ -581,36 +580,26 @@  compute_enum_values (parmpy_object *self, PyObject *enum_values)
       return 0;
     }
 
-  self->enumeration = XCNEWVEC (const char *, size + 1);
-  back_to = make_cleanup (free_current_contents, &self->enumeration);
+  gdb_argv_up enumeration (XCNEWVEC (char *, size + 1));
 
   for (i = 0; i < size; ++i)
     {
       gdbpy_ref<> item (PySequence_GetItem (enum_values, i));
 
       if (item == NULL)
-	{
-	  do_cleanups (back_to);
-	  return 0;
-	}
+	return 0;
       if (! gdbpy_is_string (item.get ()))
 	{
-	  do_cleanups (back_to);
 	  PyErr_SetString (PyExc_RuntimeError,
 			   _("The enumeration item not a string."));
 	  return 0;
 	}
-      self->enumeration[i]
-	= python_string_to_host_string (item.get ()).release ();
+      enumeration[i] = python_string_to_host_string (item.get ()).release ();
       if (self->enumeration[i] == NULL)
-	{
-	  do_cleanups (back_to);
-	  return 0;
-	}
-      make_cleanup (xfree, (char *) self->enumeration[i]);
+	return 0;
     }
 
-  discard_cleanups (back_to);
+  self->enumeration = const_cast<const char**> (enumeration.release ());
   return 1;
 }