[RFA,23/23] Use gdb_argv_up in Python
Commit Message
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
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
>>>>> "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" == 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
@@ -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.
@@ -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;
}