From 4b8cb9dd9e1fa8dc471f640c592312a751e67eed Mon Sep 17 00:00:00 2001 From: Simon Marchi Date: Wed, 12 May 2021 13:50:05 -0400 Subject: [PATCH] gdb: make gdbpy_parse_command_name return a unique_xmalloc_ptr This avoids some manual memory management. cmdpy_init correctly transfers ownership of the name to the cmd_list_element, as it sets the name_allocated flag. However, cmdpy_init (and add_setshow_generic) doesn't, it looks like the name is just leaked. This is a bit tricky, because it actually creates two commands (one set and one show), it would take a bit of refactoring of the command code to give each their own allocated copy. For now, just keep doing what the current code does but in a more explicit fashion, with an explicit 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. (add_setshow_generic): Take gdb::unique_xmalloc_ptr, release it when done. Change-Id: Iae5bc21fe2b22f12d5f954057b0aca7ca4cd3f0d --- gdb/ChangeLog | 10 +++++++ gdb/python/py-cmd.c | 47 +++++++++++++++----------------- gdb/python/py-param.c | 52 ++++++++++++++++++------------------ gdb/python/python-internal.h | 6 ++--- 4 files changed, 61 insertions(+), 54 deletions(-) diff --git a/gdb/ChangeLog b/gdb/ChangeLog index d564621f4c..fe75cee4b5 100644 --- a/gdb/ChangeLog +++ b/gdb/ChangeLog @@ -1,3 +1,13 @@ +2021-05-12 Simon Marchi + + * 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. + (add_setshow_generic): Take gdb::unique_xmalloc_ptr, release it + when done. + 2021-05-12 George Barrett * NEWS (Guile API): Note the addition of the new procedure. diff --git a/gdb/python/py-cmd.c b/gdb/python/py-cmd.c index 9833bf863b..6f7794cdf5 100644 --- a/gdb/python/py-cmd.c +++ b/gdb/python/py-cmd.c @@ -342,10 +342,10 @@ cmdpy_completer (struct cmd_list_element *command, START_LIST is the list in which the search starts. - This function returns the xmalloc()d name of the new command. On - error sets the Python error and returns NULL. */ + This function returns the name of the new command. On error sets the Python + error and returns NULL. */ -char * +gdb::unique_xmalloc_ptr gdbpy_parse_command_name (const char *name, struct cmd_list_element ***base_list, struct cmd_list_element **start_list) @@ -354,7 +354,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) @@ -369,9 +368,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 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) @@ -390,7 +390,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; } @@ -402,7 +401,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; } @@ -435,7 +433,6 @@ cmdpy_init (PyObject *self, PyObject *args, PyObject *kw) int completetype = -1; char *docstring = NULL; struct cmd_list_element **cmd_list; - char *cmd_name; static const char *keywords[] = { "name", "command_class", "completer_class", "prefix", NULL }; PyObject *is_prefix_obj = NULL; @@ -474,19 +471,18 @@ 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 cmd_name + = gdbpy_parse_command_name (name, &cmd_list, &cmdlist); + if (cmd_name == nullptr) return -1; if (is_prefix_obj != NULL) { int cmp = PyObject_IsTrue (is_prefix_obj); - if (cmp < 0) - { - xfree (cmd_name); - return -1; - } - is_prefix = cmp > 0; + if (cmp < 0) + return -1; + + is_prefix = cmp > 0; } if (PyObject_HasAttr (self, gdbpy_doc_cst)) @@ -497,10 +493,7 @@ cmdpy_init (PyObject *self, PyObject *args, PyObject *kw) { docstring = python_string_to_host_string (ds_obj.get ()).release (); if (docstring == NULL) - { - xfree (cmd_name); - return -1; - } + return -1; } } if (! docstring) @@ -519,14 +512,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, 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); + /* If successful, the above takes ownership of the name, since we set + name_allocated, so release it. */ + cmd_name.release (); + /* There appears to be no API to set this. */ cmd->func = cmdpy_function; cmd->destroyer = cmdpy_destroyer; @@ -543,7 +541,6 @@ cmdpy_init (PyObject *self, PyObject *args, PyObject *kw) } catch (const gdb_exception &except) { - xfree (cmd_name); xfree (docstring); gdbpy_convert_exception (except); return -1; diff --git a/gdb/python/py-param.c b/gdb/python/py-param.c index ab9e883dc2..99a57960c0 100644 --- a/gdb/python/py-param.c +++ b/gdb/python/py-param.c @@ -458,7 +458,8 @@ get_show_value (struct ui_file *file, int from_tty, function. */ static void add_setshow_generic (int parmclass, enum command_class cmdclass, - const char *cmd_name, parmpy_object *self, + gdb::unique_xmalloc_ptr cmd_name, + parmpy_object *self, const char *set_doc, const char *show_doc, const char *help_doc, struct cmd_list_element **set_list, @@ -471,7 +472,7 @@ add_setshow_generic (int parmclass, enum command_class cmdclass, { case var_boolean: - add_setshow_boolean_cmd (cmd_name, cmdclass, + add_setshow_boolean_cmd (cmd_name.get (), cmdclass, &self->value.boolval, set_doc, show_doc, help_doc, get_set_value, get_show_value, set_list, show_list); @@ -479,7 +480,7 @@ add_setshow_generic (int parmclass, enum command_class cmdclass, break; case var_auto_boolean: - add_setshow_auto_boolean_cmd (cmd_name, cmdclass, + add_setshow_auto_boolean_cmd (cmd_name.get (), cmdclass, &self->value.autoboolval, set_doc, show_doc, help_doc, get_set_value, get_show_value, @@ -487,26 +488,26 @@ add_setshow_generic (int parmclass, enum command_class cmdclass, break; case var_uinteger: - add_setshow_uinteger_cmd (cmd_name, cmdclass, + add_setshow_uinteger_cmd (cmd_name.get (), cmdclass, &self->value.uintval, set_doc, show_doc, help_doc, get_set_value, get_show_value, set_list, show_list); break; case var_integer: - add_setshow_integer_cmd (cmd_name, cmdclass, + add_setshow_integer_cmd (cmd_name.get (), cmdclass, &self->value.intval, set_doc, show_doc, help_doc, get_set_value, get_show_value, set_list, show_list); break; case var_string: - add_setshow_string_cmd (cmd_name, cmdclass, + add_setshow_string_cmd (cmd_name.get (), cmdclass, &self->value.stringval, set_doc, show_doc, help_doc, get_set_value, get_show_value, set_list, show_list); break; case var_string_noescape: - add_setshow_string_noescape_cmd (cmd_name, cmdclass, + add_setshow_string_noescape_cmd (cmd_name.get (), cmdclass, &self->value.stringval, set_doc, show_doc, help_doc, get_set_value, get_show_value, @@ -515,7 +516,7 @@ add_setshow_generic (int parmclass, enum command_class cmdclass, break; case var_optional_filename: - add_setshow_optional_filename_cmd (cmd_name, cmdclass, + add_setshow_optional_filename_cmd (cmd_name.get (), cmdclass, &self->value.stringval, set_doc, show_doc, help_doc, get_set_value, get_show_value, set_list, @@ -523,27 +524,27 @@ add_setshow_generic (int parmclass, enum command_class cmdclass, break; case var_filename: - add_setshow_filename_cmd (cmd_name, cmdclass, + add_setshow_filename_cmd (cmd_name.get (), cmdclass, &self->value.stringval, set_doc, show_doc, help_doc, get_set_value, get_show_value, set_list, show_list); break; case var_zinteger: - add_setshow_zinteger_cmd (cmd_name, cmdclass, + add_setshow_zinteger_cmd (cmd_name.get (), cmdclass, &self->value.intval, set_doc, show_doc, help_doc, get_set_value, get_show_value, set_list, show_list); break; case var_zuinteger: - add_setshow_zuinteger_cmd (cmd_name, cmdclass, + add_setshow_zuinteger_cmd (cmd_name.get (), cmdclass, &self->value.uintval, set_doc, show_doc, help_doc, get_set_value, get_show_value, set_list, show_list); break; case var_zuinteger_unlimited: - add_setshow_zuinteger_unlimited_cmd (cmd_name, cmdclass, + add_setshow_zuinteger_unlimited_cmd (cmd_name.get (), cmdclass, &self->value.intval, set_doc, show_doc, help_doc, get_set_value, get_show_value, @@ -551,7 +552,7 @@ add_setshow_generic (int parmclass, enum command_class cmdclass, break; case var_enum: - add_setshow_enum_cmd (cmd_name, cmdclass, self->enumeration, + add_setshow_enum_cmd (cmd_name.get (), cmdclass, self->enumeration, &self->value.cstringval, set_doc, show_doc, help_doc, get_set_value, get_show_value, set_list, show_list); @@ -562,15 +563,18 @@ add_setshow_generic (int parmclass, enum command_class cmdclass, /* Lookup created parameter, and register Python object against the parameter context. Perform this task against both lists. */ - tmp_name = cmd_name; + tmp_name = cmd_name.get (); param = lookup_cmd (&tmp_name, *show_list, "", NULL, 0, 1); if (param) set_cmd_context (param, self); - tmp_name = cmd_name; + tmp_name = cmd_name.get (); param = lookup_cmd (&tmp_name, *set_list, "", NULL, 0, 1); if (param) set_cmd_context (param, self); + + /* We (unfortunately) currently leak the command name. */ + cmd_name.release (); } /* A helper which computes enum values. Returns 1 on success. Returns 0 on @@ -657,7 +661,6 @@ parmpy_init (PyObject *self, PyObject *args, PyObject *kwds) parmpy_object *obj = (parmpy_object *) self; const char *name; gdb::unique_xmalloc_ptr 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 +709,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 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 +727,12 @@ parmpy_init (PyObject *self, PyObject *args, PyObject *kwds) try { add_setshow_generic (parmclass, (enum command_class) cmdtype, - cmd_name, obj, + std::move (cmd_name), obj, set_doc.get (), show_doc.get (), doc.get (), set_list, show_list); } 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 56702cad53..690d2fb43c 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 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.34.1