[PATCH] gdb: make gdbpy_parse_command_name return a unique_xmalloc_ptr

[Simon Marchi via Gdb-patches <gdb-patches@sourceware.org>]

This avoids some manual memory management.

The current code leaks (on purpose) the command name if adding the
command was successful.  This is now explicitly done with a release.

gdb/ChangeLog:

	* python/python-internal.h (gdbpy_parse_command_name): Return
	gdb::unique_xmalloc_ptr.
	* python/py-cmd.c (gdbpy_parse_command_name): Likewise.
	(cmdpy_init): Adjust.
	* python/py-param.c (parmpy_init): Adjust.

Change-Id: Iae5bc21fe2b22f12d5f954057b0aca7ca4cd3f0d
---
 gdb/python/py-cmd.c          | 35 +++++++++++++++++------------------
 gdb/python/py-param.c        | 22 +++++++++++-----------
 gdb/python/python-internal.h |  6 +++---
 3 files changed, 31 insertions(+), 32 deletions(-)

diff --git a/gdb/python/py-cmd.c b/gdb/python/py-cmd.c
index f4d3dcc31218..56ca4764929b 100644
--- a/gdb/python/py-cmd.c
+++ b/gdb/python/py-cmd.c
@@ -348,7 +348,7 @@ cmdpy_completer (struct cmd_list_element *command,
    This function returns the xmalloc()d name of the new command.  On
    error sets the Python error and returns NULL.  */
 
-char *
+gdb::unique_xmalloc_ptr<char>
 gdbpy_parse_command_name (const char *name,
 			  struct cmd_list_element ***base_list,
 			  struct cmd_list_element **start_list)
@@ -357,7 +357,6 @@ gdbpy_parse_command_name (const char *name,
   int len = strlen (name);
   int i, lastchar;
   const char *prefix_text2;
-  char *result;
 
   /* Skip trailing whitespace.  */
   for (i = len - 1; i >= 0 && (name[i] == ' ' || name[i] == '\t'); --i)
@@ -372,9 +371,10 @@ gdbpy_parse_command_name (const char *name,
   /* Find first character of the final word.  */
   for (; i > 0 && valid_cmd_char_p (name[i - 1]); --i)
     ;
-  result = (char *) xmalloc (lastchar - i + 2);
-  memcpy (result, &name[i], lastchar - i + 1);
-  result[lastchar - i + 1] = '\0';
+
+  gdb::unique_xmalloc_ptr<char> result ((char *) xmalloc (lastchar - i + 2));
+  memcpy (result.get (), &name[i], lastchar - i + 1);
+  result.get ()[lastchar - i + 1] = '\0';
 
   /* Skip whitespace again.  */
   for (--i; i >= 0 && (name[i] == ' ' || name[i] == '\t'); --i)
@@ -393,7 +393,6 @@ gdbpy_parse_command_name (const char *name,
     {
       PyErr_Format (PyExc_RuntimeError, _("Could not find command prefix %s."),
 		    prefix_text.c_str ());
-      xfree (result);
       return NULL;
     }
 
@@ -405,7 +404,6 @@ gdbpy_parse_command_name (const char *name,
 
   PyErr_Format (PyExc_RuntimeError, _("'%s' is not a prefix command."),
 		prefix_text.c_str ());
-  xfree (result);
   return NULL;
 }
 
@@ -438,7 +436,7 @@ cmdpy_init (PyObject *self, PyObject *args, PyObject *kw)
   int completetype = -1;
   char *docstring = NULL;
   struct cmd_list_element **cmd_list;
-  char *cmd_name, *pfx_name;
+  char *pfx_name;
   static const char *keywords[] = { "name", "command_class", "completer_class",
 				    "prefix", NULL };
   PyObject *is_prefix = NULL;
@@ -477,8 +475,9 @@ cmdpy_init (PyObject *self, PyObject *args, PyObject *kw)
       return -1;
     }
 
-  cmd_name = gdbpy_parse_command_name (name, &cmd_list, &cmdlist);
-  if (! cmd_name)
+  gdb::unique_xmalloc_ptr<char> cmd_name
+    = gdbpy_parse_command_name (name, &cmd_list, &cmdlist);
+  if (cmd_name == nullptr)
     return -1;
 
   pfx_name = NULL;
@@ -509,10 +508,7 @@ cmdpy_init (PyObject *self, PyObject *args, PyObject *kw)
 	  pfx_name[out] = '\0';
 	}
       else if (cmp < 0)
-	{
-	  xfree (cmd_name);
-	  return -1;
-	}
+	return -1;
     }
   if (PyObject_HasAttr (self, gdbpy_doc_cst))
     {
@@ -523,7 +519,6 @@ cmdpy_init (PyObject *self, PyObject *args, PyObject *kw)
 	  docstring = python_string_to_host_string (ds_obj.get ()).release ();
 	  if (docstring == NULL)
 	    {
-	      xfree (cmd_name);
 	      xfree (pfx_name);
 	      return -1;
 	    }
@@ -545,14 +540,19 @@ cmdpy_init (PyObject *self, PyObject *args, PyObject *kw)
 	  /* If we have our own "invoke" method, then allow unknown
 	     sub-commands.  */
 	  allow_unknown = PyObject_HasAttr (self, invoke_cst);
-	  cmd = add_prefix_cmd (cmd_name, (enum command_class) cmdtype,
+	  cmd = add_prefix_cmd (cmd_name.get (),
+				(enum command_class) cmdtype,
 				NULL, docstring, &obj->sub_list,
 				pfx_name, allow_unknown, cmd_list);
 	}
       else
-	cmd = add_cmd (cmd_name, (enum command_class) cmdtype,
+	cmd = add_cmd (cmd_name.get (), (enum command_class) cmdtype,
 		       docstring, cmd_list);
 
+      /* The above doesn't copy nor take ownership of the name... so we just
+         leak it.  */
+      cmd_name.release ();
+
       /* There appears to be no API to set this.  */
       cmd->func = cmdpy_function;
       cmd->destroyer = cmdpy_destroyer;
@@ -569,7 +569,6 @@ cmdpy_init (PyObject *self, PyObject *args, PyObject *kw)
     }
   catch (const gdb_exception &except)
     {
-      xfree (cmd_name);
       xfree (docstring);
       xfree (pfx_name);
       gdbpy_convert_exception (except);
diff --git a/gdb/python/py-param.c b/gdb/python/py-param.c
index ab9e883dc2d6..954b820c41ef 100644
--- a/gdb/python/py-param.c
+++ b/gdb/python/py-param.c
@@ -657,7 +657,6 @@ parmpy_init (PyObject *self, PyObject *args, PyObject *kwds)
   parmpy_object *obj = (parmpy_object *) self;
   const char *name;
   gdb::unique_xmalloc_ptr<char> set_doc, show_doc, doc;
-  char *cmd_name;
   int parmclass, cmdtype;
   PyObject *enum_values = NULL;
   struct cmd_list_element **set_list, **show_list;
@@ -706,15 +705,13 @@ parmpy_init (PyObject *self, PyObject *args, PyObject *kwds)
   obj->type = (enum var_types) parmclass;
   memset (&obj->value, 0, sizeof (obj->value));
 
-  cmd_name = gdbpy_parse_command_name (name, &set_list,
-				       &setlist);
-
-  if (! cmd_name)
+  gdb::unique_xmalloc_ptr<char> cmd_name
+    = gdbpy_parse_command_name (name, &set_list, &setlist);
+  if (cmd_name == nullptr)
     return -1;
-  xfree (cmd_name);
-  cmd_name = gdbpy_parse_command_name (name, &show_list,
-				       &showlist);
-  if (! cmd_name)
+
+  cmd_name = gdbpy_parse_command_name (name, &show_list, &showlist);
+  if (cmd_name == nullptr)
     return -1;
 
   set_doc = get_doc_string (self, set_doc_cst);
@@ -726,13 +723,16 @@ parmpy_init (PyObject *self, PyObject *args, PyObject *kwds)
   try
     {
       add_setshow_generic (parmclass, (enum command_class) cmdtype,
-			   cmd_name, obj,
+			   cmd_name.get (), obj,
 			   set_doc.get (), show_doc.get (),
 			   doc.get (), set_list, show_list);
+
+      /* The above doesn't copy nor take ownership of the name... so we just
+	 leak it.  */
+      cmd_name.release ();
     }
   catch (const gdb_exception &except)
     {
-      xfree (cmd_name);
       Py_DECREF (self);
       gdbpy_convert_exception (except);
       return -1;
diff --git a/gdb/python/python-internal.h b/gdb/python/python-internal.h
index 56702cad53a0..690d2fb43c06 100644
--- a/gdb/python/python-internal.h
+++ b/gdb/python/python-internal.h
@@ -439,9 +439,9 @@ PyObject *gdbpy_selected_thread (PyObject *self, PyObject *args);
 PyObject *gdbpy_selected_inferior (PyObject *self, PyObject *args);
 PyObject *gdbpy_string_to_argv (PyObject *self, PyObject *args);
 PyObject *gdbpy_parameter_value (enum var_types type, void *var);
-char *gdbpy_parse_command_name (const char *name,
-				struct cmd_list_element ***base_list,
-				struct cmd_list_element **start_list);
+gdb::unique_xmalloc_ptr<char> gdbpy_parse_command_name
+  (const char *name, struct cmd_list_element ***base_list,
+   struct cmd_list_element **start_list);
 PyObject *gdbpy_register_tui_window (PyObject *self, PyObject *args,
 				     PyObject *kw);
 
-- 
2.30.1