gdb: use gdb::unique_xmalloc_ptr<char> for docs in cmdpy_init
authorAndrew Burgess <aburgess@redhat.com>
Sat, 14 May 2022 09:55:59 +0000 (10:55 +0100)
committerAndrew Burgess <aburgess@redhat.com>
Sat, 28 May 2022 09:36:50 +0000 (10:36 +0100)
Make use of gdb::unique_xmalloc_ptr<char> to hold the documentation
string in cmdpy_init (when creating a custom GDB command in Python).
I think this is all pretty straight forward, the only slight weirdness
is the removal of the call to free toward the end of this function.

Prior to this commit, if an exception was thrown after the GDB command
was created then we would (I think) end up freeing the documentation
string even though the command would remain registered with GDB, which
would surely lead to undefined behaviour.

After this commit we release the doc string at the point that we hand
it over to the command creation routines.  If we throw _after_ the
command has been created within GDB then the doc string will be left
live.  If we throw during the command creation itself (either from
add_prefix_cmd or add_cmd) then it is up to those functions to free
the doc string (I suspect we don't, but I think in general the
commands are pretty bad at cleaning up after themselves, so I don't
think this is a huge problem).

gdb/python/py-cmd.c

index c0a985446192eba91c7153ddb2287846feaeff2d..bc48c2ea9f9cb3df8a96bacc232750e74f60a9a8 100644 (file)
@@ -430,7 +430,6 @@ cmdpy_init (PyObject *self, PyObject *args, PyObject *kw)
   const char *name;
   int cmdtype;
   int completetype = -1;
-  char *docstring = NULL;
   struct cmd_list_element **cmd_list;
   static const char *keywords[] = { "name", "command_class", "completer_class",
                                    "prefix", NULL };
@@ -484,19 +483,20 @@ cmdpy_init (PyObject *self, PyObject *args, PyObject *kw)
       is_prefix = cmp > 0;
     }
 
+  gdb::unique_xmalloc_ptr<char> docstring = nullptr;
   if (PyObject_HasAttr (self, gdbpy_doc_cst))
     {
       gdbpy_ref<> ds_obj (PyObject_GetAttr (self, gdbpy_doc_cst));
 
       if (ds_obj != NULL && gdbpy_is_string (ds_obj.get ()))
        {
-         docstring = python_string_to_host_string (ds_obj.get ()).release ();
-         if (docstring == NULL)
+         docstring = python_string_to_host_string (ds_obj.get ());
+         if (docstring == nullptr)
            return -1;
        }
     }
-  if (! docstring)
-    docstring = xstrdup (_("This command is not documented."));
+  if (docstring == nullptr)
+    docstring = make_unique_xstrdup (_("This command is not documented."));
 
   gdbpy_ref<> self_ref = gdbpy_ref<>::new_reference (self);
 
@@ -513,12 +513,12 @@ cmdpy_init (PyObject *self, PyObject *args, PyObject *kw)
          allow_unknown = PyObject_HasAttr (self, invoke_cst);
          cmd = add_prefix_cmd (cmd_name.get (),
                                (enum command_class) cmdtype,
-                               NULL, docstring, &obj->sub_list,
+                               NULL, docstring.release (), &obj->sub_list,
                                allow_unknown, cmd_list);
        }
       else
        cmd = add_cmd (cmd_name.get (), (enum command_class) cmdtype,
-                      docstring, cmd_list);
+                      docstring.release (), cmd_list);
 
       /* If successful, the above takes ownership of the name, since we set
          name_allocated, so release it.  */
@@ -540,7 +540,6 @@ cmdpy_init (PyObject *self, PyObject *args, PyObject *kw)
     }
   catch (const gdb_exception &except)
     {
-      xfree (docstring);
       gdbpy_convert_exception (except);
       return -1;
     }