[RFC,4/8] mi/python: C++ify python MI command handling code

Message ID 20190418152337.6376-5-jan.vrany@fit.cvut.cz
State New
Headers show
Series
  • Create MI commands using python
Related show

Commit Message

Jan Vrany April 18, 2019, 3:23 p.m.
Use gdbpy_ref<> instead of manually calling Py_DECREF().
Use gdb::unique_xmalloc_ptr<> instead of manually calling xfree().

gdb/Changelog:

	* python/py-micmd.c (parse_mi_result): Use gdb::unique_xmalloc_ptr<>
	instead of manually calling xfree().
	(py_mi_invoke): Use gdbpy_ref<> instead of PyObject * with Py_DECREF().
	Do not call Py_DECREF() on individual strings in python argument list
	object as PyList_SetItem() steals reference.
	(micmdpy_parse_command_name): Fix formatting.
	(micmdpy_init): Use gdb::unique_xmalloc_ptr<> instead of manually
	calling xfree().
---
 gdb/ChangeLog         | 11 ++++++++++
 gdb/python/py-micmd.c | 50 ++++++++++++++++---------------------------
 2 files changed, 29 insertions(+), 32 deletions(-)

-- 
2.20.1

Comments

Tom Tromey April 25, 2019, 7:43 p.m. | #1
>>>>> "Jan" == Jan Vrany <jan.vrany@fit.cvut.cz> writes:


Jan> Use gdbpy_ref<> instead of manually calling Py_DECREF().
Jan> Use gdb::unique_xmalloc_ptr<> instead of manually calling xfree().

I'm reading these in order so of course my comments are all invalidated
by reading the next patch.

I'd just roll this one into the previous one.

Tom

Patch

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index ef77fdbb5c..692b937fee 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,14 @@ 
+2018-12-11  Jan Vrany  <jan.vrany@fit.cvut.cz>
+
+	* python/py-micmd.c (parse_mi_result): Use gdb::unique_xmalloc_ptr<>
+	instead of manually calling xfree().
+	(py_mi_invoke): Use gdbpy_ref<> instead of PyObject * with Py_DECREF().
+	Do not call Py_DECREF() on individual strings in python argument list
+	object as PyList_SetItem() steals reference.
+	(micmdpy_parse_command_name): Fix formatting.
+	(micmdpy_init): Use gdb::unique_xmalloc_ptr<> instead of manually
+	calling xfree().
+
 2019-04-17  Tom Tromey  <tromey@adacore.com>
 
 	* dwarf2read.c (dwarf2_init_complex_target_type): Check "tt"
diff --git a/gdb/python/py-micmd.c b/gdb/python/py-micmd.c
index ee612e2bc5..0683a02017 100644
--- a/gdb/python/py-micmd.c
+++ b/gdb/python/py-micmd.c
@@ -53,11 +53,9 @@  parse_mi_result (PyObject *result, const char *field_name)
       ui_out_emit_list list_emitter (uiout, field_name);
       for(i = 0; i < PyList_GET_SIZE (result); ++i)
 	{
-	  //struct cleanup *cleanup_item = make_cleanup_ui_out_tuple_begin_end (uiout, NULL);
           ui_out_emit_tuple tuple_emitter (uiout, NULL);
 	  item = PyList_GetItem (result, i);
 	  parse_mi_result (item, NULL);
-	  //do_cleanups (cleanup_item);
 	}      
     }
   else if ( PyDict_Check (result))
@@ -66,9 +64,8 @@  parse_mi_result (PyObject *result, const char *field_name)
       Py_ssize_t pos = 0;
       while ( PyDict_Next (result, &pos, &key, &value) )
 	{
-	  char *key_string = gdbpy_obj_to_string (key).release ();
-	  parse_mi_result (value, key_string);
-	  xfree ( (void *) key_string);
+	  gdb::unique_xmalloc_ptr<char> key_string (gdbpy_obj_to_string (key));
+	  parse_mi_result (value, key_string.get ());
 	}
     }
 }
@@ -79,7 +76,6 @@  void
 py_mi_invoke (void *py_obj, char **argv, int argc)
 {
   micmdpy_object *obj = (micmdpy_object *) py_obj;
-  PyObject *argobj, *result,  **strings;
   int i;
 
   gdbpy_enter enter_py (get_current_arch (), current_language);
@@ -92,9 +88,8 @@  py_mi_invoke (void *py_obj, char **argv, int argc)
       error (_("Python command object missing 'invoke' method."));
     }
 
-  strings = (PyObject **) malloc (sizeof(PyObject *) * argc);
-  argobj = PyList_New (argc);
-  if ( !argobj)
+  gdbpy_ref<> argobj (PyList_New (argc));
+  if (argobj == nullptr)
     {
       gdbpy_print_stack ();
       error (_("Failed to create the python arguments list."));
@@ -102,28 +97,22 @@  py_mi_invoke (void *py_obj, char **argv, int argc)
 
   for (i = 0; i < argc; ++i)
     {
-      strings[i] = PyUnicode_Decode (argv[i], strlen(argv[i]), host_charset (), NULL);
-      if (PyList_SetItem (argobj, i, strings[i]) != 0)
+      /* Since PyList_SetItem steals the reference, we don't use
+       * gdbpy_ref<> to hold on arg string. */
+      PyObject* str  = PyUnicode_Decode (argv[i], strlen (argv[i]), host_charset (), NULL);
+      if (PyList_SetItem (argobj.get (), i, str) != 0)
 	{
 	  error (_("Failed to create the python arguments list."));
 	}
     }
 
-  result = PyObject_CallMethodObjArgs ((PyObject *) obj, invoke_cst, argobj,
-				       NULL);
+  gdbpy_ref<> result (PyObject_CallMethodObjArgs ((PyObject *) obj, invoke_cst, argobj.get (),
+				       NULL));
 
-  if (result)
+  if (result != nullptr)
     {
-      parse_mi_result (result, NULL);
-      Py_DECREF (result);
+      parse_mi_result (result.get (), NULL);
     }
-
-  Py_DECREF (argobj);
-  for (i = 0; i < argc; ++i)
-    {
-      Py_DECREF (strings[i]);
-    }
-  free (strings);
 }
 
 /* Parse the name of the MI command to register.
@@ -150,7 +139,7 @@  micmdpy_parse_command_name (const char *name)
 
   /* Skip preceding whitespaces. */
   /* Find first character of the final word.  */
-   for (; i > 0 && (isalnum (name[i - 1])
+  for (; i > 0 && (isalnum (name[i - 1])
  		   || name[i - 1] == '-'
  		   || name[i - 1] == '_');
         --i)
@@ -158,10 +147,10 @@  micmdpy_parse_command_name (const char *name)
    /* Skip the first dash to have to command name only.
     * i.e. -thread-info -> thread-info
     */
-   if(name[i] == '-' && i < len - 2)
+  if(name[i] == '-' && i < len - 2)
      i++;
 
-   if( i == lastchar)
+  if( i == lastchar)
     {
       PyErr_SetString (PyExc_RuntimeError, _("No command name found."));
       return NULL;
@@ -185,20 +174,19 @@  static int
 micmdpy_init (PyObject *self, PyObject *args, PyObject *kw)
 {
   const char *name;
-  char *cmd_name;
 
   if(! PyArg_ParseTuple (args, "s", &name))
     return -1;
 
-  cmd_name = micmdpy_parse_command_name (name);
-  if (! cmd_name)
+  gdb::unique_xmalloc_ptr<char> cmd_name (micmdpy_parse_command_name (name));
+  if (cmd_name == nullptr)
     return -1;
 
   Py_INCREF (self);
 
   try
   {
-    mi_command *micommand = new mi_command_py (cmd_name, NULL, (void *) self);
+    mi_command *micommand = new mi_command_py (cmd_name.get (), NULL, (void *) self);
 
     bool result = insert_mi_cmd_entry (mi_cmd_up (micommand));
 
@@ -211,7 +199,6 @@  micmdpy_init (PyObject *self, PyObject *args, PyObject *kw)
   }
   catch (const gdb_exception &except)
   {
-    xfree (cmd_name);
     Py_DECREF (self);
     PyErr_Format (except.reason == RETURN_QUIT
 		    ? PyExc_KeyboardInterrupt : PyExc_RuntimeError,
@@ -219,7 +206,6 @@  micmdpy_init (PyObject *self, PyObject *args, PyObject *kw)
     return -1;
   }
 
-  xfree (cmd_name);
   return 0;
 }