From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca by simark.ca with LMTP id +bQ6FaoVnGBwYgAAWB0awg (envelope-from ) for ; Wed, 12 May 2021 13:51:38 -0400 Received: by simark.ca (Postfix, from userid 112) id 438151F11C; Wed, 12 May 2021 13:51:38 -0400 (EDT) X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on simark.ca X-Spam-Level: X-Spam-Status: No, score=-1.1 required=5.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,MAILING_LIST_MULTI,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.2 Received: from sourceware.org (server2.sourceware.org [8.43.85.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPS id D4AA01E783 for ; Wed, 12 May 2021 13:51:36 -0400 (EDT) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 7CDC239B6035; Wed, 12 May 2021 17:51:36 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 7CDC239B6035 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sourceware.org; s=default; t=1620841896; bh=WufGEv6sPF2LTEtpO7djky+XHjK07YoTgPOiV0Flbz0=; h=To:Subject:Date:In-Reply-To:References:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=be02IDOvEqGDyvUqMZCN/yQwijLf6xkNmIolUFHVk/9/Xz/PY5kiIrT7NxhSmidzB DXOmPINSPucfbWPMfPVc+qpDOQ/YkCBlpO1XzAkY8PIA8nSV5EF3ZYMjdXKMiR8Vki Dr/g6qNlcbyG0FLwNYqyd1HRzsoDdxzndAKMqL60= Received: from barracuda.ebox.ca (barracuda.ebox.ca [96.127.255.19]) by sourceware.org (Postfix) with ESMTPS id CFD0B39B602C for ; Wed, 12 May 2021 17:51:33 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org CFD0B39B602C X-ASG-Debug-ID: 1620841886-0c856e6cd512a7aa0001-fS2M51 Received: from smtp.ebox.ca (smtp.ebox.ca [96.127.255.82]) by barracuda.ebox.ca with ESMTP id OGlQoYcfHAtc23bx (version=TLSv1 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Wed, 12 May 2021 13:51:26 -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 9DCE6441B21; Wed, 12 May 2021 13:51:26 -0400 (EDT) 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: [pushed] gdb: make gdbpy_parse_command_name return a unique_xmalloc_ptr Date: Wed, 12 May 2021 13:51:25 -0400 X-ASG-Orig-Subj: [pushed] gdb: make gdbpy_parse_command_name return a unique_xmalloc_ptr Message-Id: <20210512175125.2503085-1-simon.marchi@polymtl.ca> X-Mailer: git-send-email 2.30.1 In-Reply-To: <20210323183846.3886585-1-simon.marchi@polymtl.ca> References: <20210323183846.3886585-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: 1620841886 X-Barracuda-Encrypted: DHE-RSA-AES256-SHA X-Barracuda-URL: https://96.127.255.19:443/cgi-mod/mark.cgi X-Virus-Scanned: by bsmtpd at ebox.ca X-Barracuda-Scan-Msg-Size: 14218 X-Barracuda-BRTS-Status: 1 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.89890 Rule breakdown below pts rule name description ---- ---------------------- -------------------------------------------------- 0.50 BSF_RULE7568M Custom Rule 7568M 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: , From: Simon Marchi via Gdb-patches Reply-To: Simon Marchi Errors-To: gdb-patches-bounces@sourceware.org Sender: "Gdb-patches" This is the version I pushed, after rebasing. 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 d564621f4c64..fe75cee4b5e7 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 9833bf863b09..6f7794cdf53d 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 ab9e883dc2d6..99a57960c059 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