From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from barracuda.ebox.ca (barracuda.ebox.ca [96.127.255.19]) by sourceware.org (Postfix) with ESMTPS id 5F8B03858002 for ; Tue, 23 Mar 2021 18:38:48 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 5F8B03858002 X-ASG-Debug-ID: 1616524727-0c856e67e2a909d0001-fS2M51 Received: from smtp.ebox.ca (smtp.ebox.ca [96.127.255.82]) by barracuda.ebox.ca with ESMTP id T4GlL8372iktoAai (version=TLSv1 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Tue, 23 Mar 2021 14:38:47 -0400 (EDT) X-Barracuda-Envelope-From: simon.marchi@polymtl.ca X-Barracuda-RBL-Trusted-Forwarder: 96.127.255.82 Received: from simark.localdomain (192-222-157-6.qc.cable.ebox.net [192.222.157.6]) by smtp.ebox.ca (Postfix) with ESMTP id 4189C441B21; Tue, 23 Mar 2021 14:38:47 -0400 (EDT) From: Simon Marchi X-Barracuda-RBL-IP: 192.222.157.6 X-Barracuda-Effective-Source-IP: 192-222-157-6.qc.cable.ebox.net[192.222.157.6] X-Barracuda-Apparent-Source-IP: 192.222.157.6 To: gdb-patches@sourceware.org Subject: [PATCH v2] gdb: make gdbpy_parse_command_name return a unique_xmalloc_ptr Date: Tue, 23 Mar 2021 14:38:46 -0400 X-ASG-Orig-Subj: [PATCH v2] gdb: make gdbpy_parse_command_name return a unique_xmalloc_ptr Message-Id: <20210323183846.3886585-1-simon.marchi@polymtl.ca> X-Mailer: git-send-email 2.30.1 In-Reply-To: <20210323175546.3836508-1-simon.marchi@polymtl.ca> References: <20210323175546.3836508-1-simon.marchi@polymtl.ca> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Barracuda-Connect: smtp.ebox.ca[96.127.255.82] X-Barracuda-Start-Time: 1616524727 X-Barracuda-Encrypted: DHE-RSA-AES256-SHA X-Barracuda-URL: https://96.127.255.19:443/cgi-mod/mark.cgi X-Barracuda-BRTS-Status: 1 X-Virus-Scanned: by bsmtpd at ebox.ca X-Barracuda-Scan-Msg-Size: 13299 X-Barracuda-Spam-Score: 0.50 X-Barracuda-Spam-Status: No, SCORE=0.50 using global scores of TAG_LEVEL=1000.0 QUARANTINE_LEVEL=1000.0 KILL_LEVEL=8.0 tests=BSF_RULE7568M X-Barracuda-Spam-Report: Code version 3.2, rules version 3.2.3.88731 Rule breakdown below pts rule name description ---- ---------------------- -------------------------------------------------- 0.50 BSF_RULE7568M Custom Rule 7568M X-Spam-Status: No, score=-20.7 required=5.0 tests=BAYES_00, GIT_PATCH_0, KAM_DMARC_QUARANTINE, KAM_DMARC_STATUS, RCVD_IN_DNSWL_LOW, SPF_HELO_NONE, SPF_SOFTFAIL, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 23 Mar 2021 18:38:50 -0000 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/python/py-cmd.c | 35 ++++++++++++------------ gdb/python/py-param.c | 52 ++++++++++++++++++------------------ gdb/python/python-internal.h | 6 ++--- 3 files changed, 46 insertions(+), 47 deletions(-) diff --git a/gdb/python/py-cmd.c b/gdb/python/py-cmd.c index f4d3dcc31218..eaaa5ea84a9b 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 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 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 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); + /* 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; @@ -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..71fb9a53595a 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 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 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