* [PATCH 0/2] make xstrprintf and xstrvprintf return a unique_ptr
@ 2021-11-08 17:11 Andrew Burgess
2021-11-08 17:11 ` [PATCH 1/2] gdbsupport: move xfree into its own file Andrew Burgess
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Andrew Burgess @ 2021-11-08 17:11 UTC (permalink / raw)
To: gdb-patches; +Cc: Andrew Burgess
I thought it might be a good idea if xstrprintf return a
gdb::unique_xmalloc_ptr<char>. It removes the need to manually call
free in a few places and avoids us having to repack the char* into a
unique_xmalloc_ptr in a few more places.
There are plenty of places where I just end up calling '.release()' on
the result, but I prefer that as it makes it explicit that the
allocated pointer is "escaping". I'd hope, over time, we'd slowly
remove these cases from GDB and use more managed pointers.
Thoughts?
Thanks,
Andrew
---
Andrew Burgess (2):
gdbsupport: move xfree into its own file
gdb/gdbsupport: make xstrprintf and xstrvprintf return a unique_ptr
gdb/ada-lang.c | 3 +--
gdb/auto-load.c | 17 ++++++---------
gdb/breakpoint.c | 9 ++++----
gdb/charset.c | 18 +++++++---------
gdb/cli/cli-cmds.c | 9 ++++----
gdb/cli/cli-decode.c | 12 +++++------
gdb/cli/cli-utils.c | 2 +-
gdb/cp-abi.c | 11 ++++------
gdb/frv-tdep.c | 14 ++++---------
gdb/gcore.c | 2 +-
gdb/guile/scm-breakpoint.c | 2 +-
gdb/guile/scm-cmd.c | 33 +++++++++++++----------------
gdb/guile/scm-exception.c | 12 +++++------
gdb/guile/scm-gsmob.c | 7 ++-----
gdb/guile/scm-ports.c | 6 +++---
gdb/jit.c | 4 ++--
gdb/language.c | 3 +--
gdb/linespec.c | 5 +++--
gdb/location.c | 9 +++++---
gdb/macrotab.c | 2 +-
gdb/main.c | 2 +-
gdb/mi/mi-cmd-env.c | 2 +-
gdb/python/py-breakpoint.c | 2 +-
gdb/remote.c | 39 +++++++++++++++--------------------
gdb/riscv-tdep.c | 4 ++--
gdb/rust-lang.h | 5 ++---
gdb/skip.c | 2 +-
gdb/target.c | 6 +++---
gdb/tracepoint.c | 2 +-
gdb/tui/tui-layout.c | 4 ++--
gdb/utils.c | 4 ++--
gdb/xtensa-tdep.c | 3 ++-
gdbsupport/common-utils.cc | 10 ++++-----
gdbsupport/common-utils.h | 22 ++++----------------
gdbsupport/gdb-xfree.h | 41 +++++++++++++++++++++++++++++++++++++
gdbsupport/gdb_unique_ptr.h | 1 +
36 files changed, 162 insertions(+), 167 deletions(-)
create mode 100644 gdbsupport/gdb-xfree.h
--
2.25.4
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH 1/2] gdbsupport: move xfree into its own file
2021-11-08 17:11 [PATCH 0/2] make xstrprintf and xstrvprintf return a unique_ptr Andrew Burgess
@ 2021-11-08 17:11 ` Andrew Burgess
2021-11-08 17:11 ` [PATCH 2/2] gdb/gdbsupport: make xstrprintf and xstrvprintf return a unique_ptr Andrew Burgess
2021-11-08 18:11 ` [PATCH 0/2] " Tom Tromey
2 siblings, 0 replies; 4+ messages in thread
From: Andrew Burgess @ 2021-11-08 17:11 UTC (permalink / raw)
To: gdb-patches; +Cc: Andrew Burgess
In the next commit I'd like to reference gdb_unique_ptr within the
common-utils.h file. However, this requires that I include
gdb_unique_ptr.h, which requires that xfree be defined.
Interestingly, gdb_unique_ptr.h doesn't actually include anything that
defines xfree, but I was finding that when I added a gdb_unique_ptr.h
include to common-utils.h I was getting a dependency cycle; before my
change xfree was defined when gdb_unique_ptr.h was processed, while
after my change it was not, and this made g++ unhappy.
To break this cycle, I propose to move xfree into its own header file,
gdb-xfree.h, which I'll then include into gdb_unique_ptr.h and
common-utils.cc.
---
gdbsupport/common-utils.cc | 1 +
gdbsupport/common-utils.h | 16 ---------------
gdbsupport/gdb-xfree.h | 41 +++++++++++++++++++++++++++++++++++++
gdbsupport/gdb_unique_ptr.h | 1 +
4 files changed, 43 insertions(+), 16 deletions(-)
create mode 100644 gdbsupport/gdb-xfree.h
diff --git a/gdbsupport/common-utils.cc b/gdbsupport/common-utils.cc
index 8ce839a54ae..7a8434538dc 100644
--- a/gdbsupport/common-utils.cc
+++ b/gdbsupport/common-utils.cc
@@ -21,6 +21,7 @@
#include "common-utils.h"
#include "host-defs.h"
#include "safe-ctype.h"
+#include "gdbsupport/gdb-xfree.h"
void *
xzalloc (size_t size)
diff --git a/gdbsupport/common-utils.h b/gdbsupport/common-utils.h
index 224e1f31222..4a75e67079f 100644
--- a/gdbsupport/common-utils.h
+++ b/gdbsupport/common-utils.h
@@ -52,22 +52,6 @@
/* Like xmalloc, but zero the memory. */
void *xzalloc (size_t);
-template <typename T>
-static void
-xfree (T *ptr)
-{
- static_assert (IsFreeable<T>::value, "Trying to use xfree with a non-POD \
-data type. Use operator delete instead.");
-
- if (ptr != NULL)
-#ifdef GNULIB_NAMESPACE
- GNULIB_NAMESPACE::free (ptr); /* ARI: free */
-#else
- free (ptr); /* ARI: free */
-#endif
-}
-
-
/* Like asprintf and vasprintf, but return the string, throw an error
if no memory. */
char *xstrprintf (const char *format, ...) ATTRIBUTE_PRINTF (1, 2);
diff --git a/gdbsupport/gdb-xfree.h b/gdbsupport/gdb-xfree.h
new file mode 100644
index 00000000000..2028698564e
--- /dev/null
+++ b/gdbsupport/gdb-xfree.h
@@ -0,0 +1,41 @@
+/* Copyright (C) 1986-2021 Free Software Foundation, Inc.
+
+ This file is part of GDB.
+
+ This program is free software; you can redistribute it and/or modify
+ it under the terms of the GNU General Public License as published by
+ the Free Software Foundation; either version 3 of the License, or
+ (at your option) any later version.
+
+ This program is distributed in the hope that it will be useful,
+ but WITHOUT ANY WARRANTY; without even the implied warranty of
+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ GNU General Public License for more details.
+
+ You should have received a copy of the GNU General Public License
+ along with this program. If not, see <http://www.gnu.org/licenses/>. */
+
+#ifndef GDBSUPPORT_GDB_XFREE_H
+#define GDBSUPPORT_GDB_XFREE_H
+
+#include "gdbsupport/poison.h"
+
+/* GDB uses this instead of 'free', it detects when it is called on an
+ invalid type. */
+
+template <typename T>
+static void
+xfree (T *ptr)
+{
+ static_assert (IsFreeable<T>::value, "Trying to use xfree with a non-POD \
+data type. Use operator delete instead.");
+
+ if (ptr != NULL)
+#ifdef GNULIB_NAMESPACE
+ GNULIB_NAMESPACE::free (ptr); /* ARI: free */
+#else
+ free (ptr); /* ARI: free */
+#endif
+}
+
+#endif /* GDBSUPPORT_GDB_XFREE_H */
diff --git a/gdbsupport/gdb_unique_ptr.h b/gdbsupport/gdb_unique_ptr.h
index 77aecb48e62..a4904ead57c 100644
--- a/gdbsupport/gdb_unique_ptr.h
+++ b/gdbsupport/gdb_unique_ptr.h
@@ -21,6 +21,7 @@
#define COMMON_GDB_UNIQUE_PTR_H
#include <memory>
+#include "gdbsupport/gdb-xfree.h"
namespace gdb
{
--
2.25.4
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH 2/2] gdb/gdbsupport: make xstrprintf and xstrvprintf return a unique_ptr
2021-11-08 17:11 [PATCH 0/2] make xstrprintf and xstrvprintf return a unique_ptr Andrew Burgess
2021-11-08 17:11 ` [PATCH 1/2] gdbsupport: move xfree into its own file Andrew Burgess
@ 2021-11-08 17:11 ` Andrew Burgess
2021-11-08 18:11 ` [PATCH 0/2] " Tom Tromey
2 siblings, 0 replies; 4+ messages in thread
From: Andrew Burgess @ 2021-11-08 17:11 UTC (permalink / raw)
To: gdb-patches; +Cc: Andrew Burgess
The motivation is to reduce the number of places where unmanaged
pointers are returned from allocation type routines. All of the
callers are updated.
There should be no user visible changes after this commit.
---
gdb/ada-lang.c | 3 +--
gdb/auto-load.c | 17 ++++++-----------
gdb/breakpoint.c | 9 ++++-----
gdb/charset.c | 18 +++++++-----------
gdb/cli/cli-cmds.c | 9 ++++-----
gdb/cli/cli-decode.c | 12 ++++++------
gdb/cli/cli-utils.c | 2 +-
gdb/cp-abi.c | 11 ++++-------
gdb/frv-tdep.c | 14 ++++----------
gdb/gcore.c | 2 +-
gdb/guile/scm-breakpoint.c | 2 +-
gdb/guile/scm-cmd.c | 33 ++++++++++++++------------------
gdb/guile/scm-exception.c | 12 +++++-------
gdb/guile/scm-gsmob.c | 7 ++-----
gdb/guile/scm-ports.c | 6 +++---
gdb/jit.c | 4 ++--
gdb/language.c | 3 +--
gdb/linespec.c | 5 +++--
gdb/location.c | 9 ++++++---
gdb/macrotab.c | 2 +-
gdb/main.c | 2 +-
gdb/mi/mi-cmd-env.c | 2 +-
gdb/python/py-breakpoint.c | 2 +-
gdb/remote.c | 39 +++++++++++++++++---------------------
gdb/riscv-tdep.c | 4 ++--
gdb/rust-lang.h | 5 ++---
gdb/skip.c | 2 +-
gdb/target.c | 6 +++---
gdb/tracepoint.c | 2 +-
gdb/tui/tui-layout.c | 4 ++--
gdb/utils.c | 4 ++--
gdb/xtensa-tdep.c | 3 ++-
gdbsupport/common-utils.cc | 9 ++++-----
gdbsupport/common-utils.h | 6 ++++--
34 files changed, 119 insertions(+), 151 deletions(-)
diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index d964dae42cf..f04364c1735 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -13287,8 +13287,7 @@ class ada_language : public language_defn
{
type = check_typedef (TYPE_TARGET_TYPE (check_typedef (type)));
std::string name = type_to_string (type);
- return gdb::unique_xmalloc_ptr<char>
- (xstrprintf ("{%s} %s", name.c_str (), core_addr_to_string (addr)));
+ return xstrprintf ("{%s} %s", name.c_str (), core_addr_to_string (addr));
}
/* See language.h. */
diff --git a/gdb/auto-load.c b/gdb/auto-load.c
index 36d87252b3f..6579eb57ffb 100644
--- a/gdb/auto-load.c
+++ b/gdb/auto-load.c
@@ -1489,8 +1489,8 @@ void
_initialize_auto_load ()
{
struct cmd_list_element *cmd;
- char *scripts_directory_help, *gdb_name_help, *python_name_help;
- char *guile_name_help;
+ gdb::unique_xmalloc_ptr<char> scripts_directory_help, gdb_name_help,
+ python_name_help, guile_name_help;
const char *suffix;
gdb::observers::new_objfile.attach (auto_load_new_objfile,
@@ -1565,23 +1565,18 @@ having 'set auto-load ... off'.\n\
Directories listed here need to be present also \
in the 'set auto-load safe-path'\n\
option."),
- gdb_name_help,
- python_name_help ? python_name_help : "",
- guile_name_help ? guile_name_help : "");
+ gdb_name_help.get (),
+ python_name_help.get () ? python_name_help.get () : "",
+ guile_name_help.get () ? guile_name_help.get () : "");
add_setshow_optional_filename_cmd ("scripts-directory", class_support,
&auto_load_dir, _("\
Set the list of directories from which to load auto-loaded scripts."), _("\
Show the list of directories from which to load auto-loaded scripts."),
- scripts_directory_help,
+ scripts_directory_help.get (),
set_auto_load_dir, show_auto_load_dir,
auto_load_set_cmdlist_get (),
auto_load_show_cmdlist_get ());
- xfree (scripts_directory_help);
- xfree (python_name_help);
- xfree (gdb_name_help);
- xfree (guile_name_help);
-
auto_load_safe_path_vec_update ();
add_setshow_optional_filename_cmd ("safe-path", class_support,
&auto_load_safe_path, _("\
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 4ec60f334a2..fc4ead82a44 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -8728,7 +8728,7 @@ static void
update_dprintf_command_list (struct breakpoint *b)
{
const char *dprintf_args = b->extra_string.get ();
- char *printf_line = NULL;
+ gdb::unique_xmalloc_ptr<char> printf_line = nullptr;
if (!dprintf_args)
return;
@@ -8779,7 +8779,7 @@ update_dprintf_command_list (struct breakpoint *b)
/* Manufacture a printf sequence. */
struct command_line *printf_cmd_line
- = new struct command_line (simple_control, printf_line);
+ = new struct command_line (simple_control, printf_line.release ());
breakpoint_set_commands (b, counted_command_line (printf_cmd_line,
command_lines_deleter ()));
}
@@ -10798,9 +10798,8 @@ watch_command_1 (const char *arg, int accessflag, int from_tty,
w->exp_string_reparse
= current_language->watch_location_expression (t, addr);
- w->exp_string.reset (xstrprintf ("-location %.*s",
- (int) (exp_end - exp_start),
- exp_start));
+ w->exp_string = xstrprintf ("-location %.*s",
+ (int) (exp_end - exp_start), exp_start);
}
else
w->exp_string.reset (savestring (exp_start, exp_end - exp_start));
diff --git a/gdb/charset.c b/gdb/charset.c
index 9fb1a1f0b67..d1aebaa5011 100644
--- a/gdb/charset.c
+++ b/gdb/charset.c
@@ -960,35 +960,31 @@ intermediate_encoding (void)
{
iconv_t desc;
static const char *stored_result = NULL;
- char *result;
+ gdb::unique_xmalloc_ptr<char> result;
if (stored_result)
return stored_result;
result = xstrprintf ("UTF-%d%s", (int) (sizeof (gdb_wchar_t) * 8),
ENDIAN_SUFFIX);
/* Check that the name is supported by iconv_open. */
- desc = iconv_open (result, host_charset ());
+ desc = iconv_open (result.get (), host_charset ());
if (desc != (iconv_t) -1)
{
iconv_close (desc);
- stored_result = result;
- return result;
+ stored_result = result.release ();
+ return stored_result;
}
- /* Not valid, free the allocated memory. */
- xfree (result);
/* Second try, with UCS-2 type. */
result = xstrprintf ("UCS-%d%s", (int) sizeof (gdb_wchar_t),
ENDIAN_SUFFIX);
/* Check that the name is supported by iconv_open. */
- desc = iconv_open (result, host_charset ());
+ desc = iconv_open (result.get (), host_charset ());
if (desc != (iconv_t) -1)
{
iconv_close (desc);
- stored_result = result;
- return result;
+ stored_result = result.release ();
+ return stored_result;
}
- /* Not valid, free the allocated memory. */
- xfree (result);
/* No valid charset found, generate error here. */
error (_("Unable to find a valid charset for string conversions"));
}
diff --git a/gdb/cli/cli-cmds.c b/gdb/cli/cli-cmds.c
index 2cf614c8e7e..e878f6ee1c0 100644
--- a/gdb/cli/cli-cmds.c
+++ b/gdb/cli/cli-cmds.c
@@ -943,7 +943,6 @@ edit_command (const char *arg, int from_tty)
struct symtab_and_line sal;
struct symbol *sym;
const char *editor;
- char *p;
const char *fn;
/* Pull in the current default source line if necessary. */
@@ -1032,9 +1031,9 @@ edit_command (const char *arg, int from_tty)
/* Quote the file name, in case it has whitespace or other special
characters. */
- p = xstrprintf ("%s +%d \"%s\"", editor, sal.line, fn);
- shell_escape (p, from_tty);
- xfree (p);
+ gdb::unique_xmalloc_ptr<char> p
+ = xstrprintf ("%s +%d \"%s\"", editor, sal.line, fn);
+ shell_escape (p.get (), from_tty);
}
/* The options for the "pipe" command. */
@@ -2660,7 +2659,7 @@ Usage: source [-s] [-v] FILE\n\
-v: each command in FILE is echoed as it is executed.\n\
\n\
Note that the file \"%s\" is read automatically in this way\n\
-when GDB is started."), GDBINIT);
+when GDB is started."), GDBINIT).release ();
c = add_cmd ("source", class_support, source_command,
source_help_text, &cmdlist);
set_cmd_completer (c, filename_completer);
diff --git a/gdb/cli/cli-decode.c b/gdb/cli/cli-decode.c
index 667d1558575..030cba44338 100644
--- a/gdb/cli/cli-decode.c
+++ b/gdb/cli/cli-decode.c
@@ -534,8 +534,8 @@ add_setshow_cmd_full_erased (const char *name,
{
struct cmd_list_element *set;
struct cmd_list_element *show;
- char *full_set_doc;
- char *full_show_doc;
+ gdb::unique_xmalloc_ptr<char> full_set_doc;
+ gdb::unique_xmalloc_ptr<char> full_show_doc;
if (help_doc != NULL)
{
@@ -544,18 +544,18 @@ add_setshow_cmd_full_erased (const char *name,
}
else
{
- full_set_doc = xstrdup (set_doc);
- full_show_doc = xstrdup (show_doc);
+ full_set_doc = make_unique_xstrdup (set_doc);
+ full_show_doc = make_unique_xstrdup (show_doc);
}
set = add_set_or_show_cmd (name, set_cmd, theclass, var_type, args,
- full_set_doc, set_list);
+ full_set_doc.release (), set_list);
set->doc_allocated = 1;
if (set_func != NULL)
set->func = set_func;
show = add_set_or_show_cmd (name, show_cmd, theclass, var_type, args,
- full_show_doc, show_list);
+ full_show_doc.release (), show_list);
show->doc_allocated = 1;
show->show_value_func = show_func;
/* Disable the default symbol completer. Doesn't make much sense
diff --git a/gdb/cli/cli-utils.c b/gdb/cli/cli-utils.c
index 3ee6ca69272..8fc7557dbfe 100644
--- a/gdb/cli/cli-utils.c
+++ b/gdb/cli/cli-utils.c
@@ -209,7 +209,7 @@ The flag -q disables the production of these headers and messages.%s"),
prefix, entity_kind, entity_kind, entity_kind,
(document_n_flag ? _("\n\
By default, the command will include non-debug symbols in the output;\n\
-these can be excluded using the -n flag.") : ""));
+these can be excluded using the -n flag.") : "")).release ();
}
/* See documentation in cli-utils.h. */
diff --git a/gdb/cp-abi.c b/gdb/cp-abi.c
index f831b2b34b3..f14bf45b04d 100644
--- a/gdb/cp-abi.c
+++ b/gdb/cp-abi.c
@@ -265,7 +265,6 @@ register_cp_abi (struct cp_abi_ops *abi)
void
set_cp_abi_as_auto_default (const char *short_name)
{
- char *new_longname, *new_doc;
struct cp_abi_ops *abi = find_cp_abi (short_name);
if (abi == NULL)
@@ -279,12 +278,10 @@ set_cp_abi_as_auto_default (const char *short_name)
auto_cp_abi = *abi;
auto_cp_abi.shortname = "auto";
- new_longname = xstrprintf ("currently \"%s\"", abi->shortname);
- auto_cp_abi.longname = new_longname;
-
- new_doc = xstrprintf ("Automatically selected; currently \"%s\"",
- abi->shortname);
- auto_cp_abi.doc = new_doc;
+ auto_cp_abi.longname = xstrprintf ("currently \"%s\"",
+ abi->shortname).release ();
+ auto_cp_abi.doc = xstrprintf ("Automatically selected; currently \"%s\"",
+ abi->shortname).release ();
/* Since we copy the current ABI into current_cp_abi instead of
using a pointer, if auto is currently the default, we need to
diff --git a/gdb/frv-tdep.c b/gdb/frv-tdep.c
index e105c27ae82..40d62f5befc 100644
--- a/gdb/frv-tdep.c
+++ b/gdb/frv-tdep.c
@@ -182,11 +182,8 @@ new_variant (void)
in the G packet. If we need more in the future, we'll add them
elsewhere. */
for (r = acc0_regnum; r <= acc7_regnum; r++)
- {
- char *buf;
- buf = xstrprintf ("acc%d", r - acc0_regnum);
- var->register_names[r] = buf;
- }
+ var->register_names[r]
+ = xstrprintf ("acc%d", r - acc0_regnum).release ();
/* accg0 - accg7: These are one byte registers. The remote protocol
provides the raw values packed four into a slot. accg0123 and
@@ -195,11 +192,8 @@ new_variant (void)
likely not want to see these raw values. */
for (r = accg0_regnum; r <= accg7_regnum; r++)
- {
- char *buf;
- buf = xstrprintf ("accg%d", r - accg0_regnum);
- var->register_names[r] = buf;
- }
+ var->register_names[r]
+ = xstrprintf ("accg%d", r - accg0_regnum).release ();
/* msr0 and msr1. */
diff --git a/gdb/gcore.c b/gdb/gcore.c
index dccb77052ba..0b4bab6e67d 100644
--- a/gdb/gcore.c
+++ b/gdb/gcore.c
@@ -136,7 +136,7 @@ gcore_command (const char *args, int from_tty)
else
{
/* Default corefile name is "core.PID". */
- corefilename.reset (xstrprintf ("core.%d", inferior_ptid.pid ()));
+ corefilename = xstrprintf ("core.%d", inferior_ptid.pid ());
}
if (info_verbose)
diff --git a/gdb/guile/scm-breakpoint.c b/gdb/guile/scm-breakpoint.c
index ab1ddb1bae0..15ac45a39fc 100644
--- a/gdb/guile/scm-breakpoint.c
+++ b/gdb/guile/scm-breakpoint.c
@@ -968,7 +968,7 @@ gdbscm_set_breakpoint_stop_x (SCM self, SCM newvalue)
= xstrprintf (_("Only one stop condition allowed. There is"
" currently a %s stop condition defined for"
" this breakpoint."),
- ext_lang_capitalized_name (extlang));
+ ext_lang_capitalized_name (extlang)).release ();
scm_dynwind_begin ((scm_t_dynwind_flags) 0);
gdbscm_dynwind_xfree (error_text);
diff --git a/gdb/guile/scm-cmd.c b/gdb/guile/scm-cmd.c
index 676428577b2..619c89c252c 100644
--- a/gdb/guile/scm-cmd.c
+++ b/gdb/guile/scm-cmd.c
@@ -475,9 +475,7 @@ gdbscm_parse_command_name (const char *name,
struct cmd_list_element *elt;
int len = strlen (name);
int i, lastchar;
- char *prefix_text;
- const char *prefix_text2;
- char *result, *msg;
+ char *msg;
/* Skip trailing whitespace. */
for (i = len - 1; i >= 0 && (name[i] == ' ' || name[i] == '\t'); --i)
@@ -493,9 +491,9 @@ gdbscm_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)
@@ -503,20 +501,19 @@ gdbscm_parse_command_name (const char *name,
if (i < 0)
{
*base_list = start_list;
- return result;
+ return result.release ();
}
- prefix_text = (char *) xmalloc (i + 2);
- memcpy (prefix_text, name, i + 1);
- prefix_text[i + 1] = '\0';
+ gdb::unique_xmalloc_ptr<char> prefix_text ((char *) xmalloc (i + 2));
+ memcpy (prefix_text.get (), name, i + 1);
+ prefix_text.get ()[i + 1] = '\0';
- prefix_text2 = prefix_text;
+ const char *prefix_text2 = prefix_text.get ();
elt = lookup_cmd_1 (&prefix_text2, *start_list, NULL, NULL, 1);
if (elt == NULL || elt == CMD_LIST_AMBIGUOUS)
{
- msg = xstrprintf (_("could not find command prefix '%s'"), prefix_text);
- xfree (prefix_text);
- xfree (result);
+ msg = xstrprintf (_("could not find command prefix '%s'"),
+ prefix_text.get ()).release ();
scm_dynwind_begin ((scm_t_dynwind_flags) 0);
gdbscm_dynwind_xfree (msg);
gdbscm_out_of_range_error (func_name, arg_pos,
@@ -525,14 +522,12 @@ gdbscm_parse_command_name (const char *name,
if (elt->is_prefix ())
{
- xfree (prefix_text);
*base_list = elt->subcommands;
- return result;
+ return result.release ();
}
- msg = xstrprintf (_("'%s' is not a prefix command"), prefix_text);
- xfree (prefix_text);
- xfree (result);
+ msg = xstrprintf (_("'%s' is not a prefix command"),
+ prefix_text.get ()).release ();
scm_dynwind_begin ((scm_t_dynwind_flags) 0);
gdbscm_dynwind_xfree (msg);
gdbscm_out_of_range_error (func_name, arg_pos,
diff --git a/gdb/guile/scm-exception.c b/gdb/guile/scm-exception.c
index b62eaebfda6..0c9cb5d50ef 100644
--- a/gdb/guile/scm-exception.c
+++ b/gdb/guile/scm-exception.c
@@ -234,7 +234,7 @@ SCM
gdbscm_make_type_error (const char *subr, int arg_pos, SCM bad_value,
const char *expected_type)
{
- char *msg;
+ gdb::unique_xmalloc_ptr<char> msg;
SCM result;
if (arg_pos > 0)
@@ -262,9 +262,8 @@ gdbscm_make_type_error (const char *subr, int arg_pos, SCM bad_value,
msg = xstrprintf (_("Wrong type argument: ~S"));
}
- result = gdbscm_make_error (scm_arg_type_key, subr, msg,
+ result = gdbscm_make_error (scm_arg_type_key, subr, msg.get (),
scm_list_1 (bad_value), scm_list_1 (bad_value));
- xfree (msg);
return result;
}
@@ -279,7 +278,7 @@ static SCM
gdbscm_make_arg_error (SCM key, const char *subr, int arg_pos, SCM bad_value,
const char *error_prefix, const char *error)
{
- char *msg;
+ gdb::unique_xmalloc_ptr<char> msg;
SCM result;
if (error_prefix != NULL)
@@ -300,9 +299,8 @@ gdbscm_make_arg_error (SCM key, const char *subr, int arg_pos, SCM bad_value,
msg = xstrprintf (_("%s: ~S"), error);
}
- result = gdbscm_make_error (key, subr, msg,
- scm_list_1 (bad_value), scm_list_1 (bad_value));
- xfree (msg);
+ result = gdbscm_make_error (key, subr, msg.get (), scm_list_1 (bad_value),
+ scm_list_1 (bad_value));
return result;
}
diff --git a/gdb/guile/scm-gsmob.c b/gdb/guile/scm-gsmob.c
index 72a96a781c1..110c1e69593 100644
--- a/gdb/guile/scm-gsmob.c
+++ b/gdb/guile/scm-gsmob.c
@@ -191,16 +191,13 @@ gdbscm_gsmob_kind (SCM self)
SCM smob, result;
scm_t_bits smobnum;
const char *name;
- char *kind;
smob = gsscm_get_gsmob_arg_unsafe (self, SCM_ARG1, FUNC_NAME);
smobnum = SCM_SMOBNUM (smob);
name = SCM_SMOBNAME (smobnum);
- kind = xstrprintf ("<%s>", name);
- result = scm_from_latin1_symbol (kind);
- xfree (kind);
-
+ gdb::unique_xmalloc_ptr<char> kind = xstrprintf ("<%s>", name);
+ result = scm_from_latin1_symbol (kind.get ());
return result;
}
diff --git a/gdb/guile/scm-ports.c b/gdb/guile/scm-ports.c
index 6251e697e64..41ccf30202d 100644
--- a/gdb/guile/scm-ports.c
+++ b/gdb/guile/scm-ports.c
@@ -139,7 +139,7 @@ static const unsigned min_memory_port_buf_size = 1;
static const unsigned max_memory_port_buf_size = 4096;
/* "out of range" error message for buf sizes. */
-static char *out_of_range_buf_size;
+static gdb::unique_xmalloc_ptr<char> out_of_range_buf_size;
#else
@@ -1447,7 +1447,7 @@ gdbscm_set_memory_port_read_buffer_size_x (SCM port, SCM size)
max_memory_port_buf_size))
{
gdbscm_out_of_range_error (FUNC_NAME, SCM_ARG2, size,
- out_of_range_buf_size);
+ out_of_range_buf_size.get ());
}
iomem = (ioscm_memory_port *) SCM_STREAM (port);
@@ -1497,7 +1497,7 @@ gdbscm_set_memory_port_write_buffer_size_x (SCM port, SCM size)
max_memory_port_buf_size))
{
gdbscm_out_of_range_error (FUNC_NAME, SCM_ARG2, size,
- out_of_range_buf_size);
+ out_of_range_buf_size.get ());
}
iomem = (ioscm_memory_port *) SCM_STREAM (port);
diff --git a/gdb/jit.c b/gdb/jit.c
index 666262e2a05..e190e695aa8 100644
--- a/gdb/jit.c
+++ b/gdb/jit.c
@@ -164,8 +164,8 @@ jit_reader_load_command (const char *args, int from_tty)
error (_("JIT reader already loaded. Run jit-reader-unload first."));
if (!IS_ABSOLUTE_PATH (file.get ()))
- file.reset (xstrprintf ("%s%s%s", jit_reader_dir.c_str (), SLASH_STRING,
- file.get ()));
+ file = xstrprintf ("%s%s%s", jit_reader_dir.c_str (),
+ SLASH_STRING, file.get ());
loaded_jit_reader = jit_reader_load (file.get ());
reinit_frame_cache ();
diff --git a/gdb/language.c b/gdb/language.c
index 9ac48a494c9..3e60a668ed6 100644
--- a/gdb/language.c
+++ b/gdb/language.c
@@ -602,8 +602,7 @@ language_defn::watch_location_expression (struct type *type,
/* Generates an expression that assumes a C like syntax is valid. */
type = check_typedef (TYPE_TARGET_TYPE (check_typedef (type)));
std::string name = type_to_string (type);
- return gdb::unique_xmalloc_ptr<char>
- (xstrprintf ("* (%s *) %s", name.c_str (), core_addr_to_string (addr)));
+ return xstrprintf ("* (%s *) %s", name.c_str (), core_addr_to_string (addr));
}
/* See language.h. */
diff --git a/gdb/linespec.c b/gdb/linespec.c
index d088801952a..56bfaede9d9 100644
--- a/gdb/linespec.c
+++ b/gdb/linespec.c
@@ -1078,11 +1078,12 @@ add_sal_to_sals (struct linespec_state *self,
the time being. */
if (symname != NULL && sal->line != 0
&& self->language->la_language == language_ada)
- canonical->suffix = xstrprintf ("%s:%d", symname, sal->line);
+ canonical->suffix = xstrprintf ("%s:%d", symname,
+ sal->line).release ();
else if (symname != NULL)
canonical->suffix = xstrdup (symname);
else
- canonical->suffix = xstrprintf ("%d", sal->line);
+ canonical->suffix = xstrprintf ("%d", sal->line).release ();
canonical->symtab = sal->symtab;
}
else
diff --git a/gdb/location.c b/gdb/location.c
index 827294e106a..33e5805459f 100644
--- a/gdb/location.c
+++ b/gdb/location.c
@@ -415,9 +415,12 @@ event_location_to_string (struct event_location *location)
break;
case ADDRESS_LOCATION:
- EL_STRING (location)
- = xstrprintf ("*%s",
- core_addr_to_string (EL_ADDRESS (location)));
+ {
+ const char *addr_string
+ = core_addr_to_string (EL_ADDRESS (location));
+ EL_STRING (location)
+ = xstrprintf ("*%s", addr_string).release ();
+ }
break;
case EXPLICIT_LOCATION:
diff --git a/gdb/macrotab.c b/gdb/macrotab.c
index 96b29e345aa..bf2d79d219f 100644
--- a/gdb/macrotab.c
+++ b/gdb/macrotab.c
@@ -893,7 +893,7 @@ fixup_definition (const char *filename, int line, struct macro_definition *def)
}
else if (def->argc == macro_LINE)
{
- saved_expansion.reset (xstrprintf ("%d", line));
+ saved_expansion = xstrprintf ("%d", line);
def->replacement = saved_expansion.get ();
}
}
diff --git a/gdb/main.c b/gdb/main.c
index ca4ccc375dc..5e97f98e928 100644
--- a/gdb/main.c
+++ b/gdb/main.c
@@ -702,7 +702,7 @@ captured_main_1 (struct captured_main_args *context)
/* Prefix warning messages with the command name. */
gdb::unique_xmalloc_ptr<char> tmp_warn_preprint
- (xstrprintf ("%s: warning: ", gdb_program_name));
+ = xstrprintf ("%s: warning: ", gdb_program_name);
warning_pre_print = tmp_warn_preprint.get ();
current_directory = getcwd (NULL, 0);
diff --git a/gdb/mi/mi-cmd-env.c b/gdb/mi/mi-cmd-env.c
index f9685a515ad..078087f327e 100644
--- a/gdb/mi/mi-cmd-env.c
+++ b/gdb/mi/mi-cmd-env.c
@@ -48,7 +48,7 @@ env_execute_cli_command (const char *cmd, const char *args)
gdb::unique_xmalloc_ptr<char> run;
if (args != NULL)
- run.reset (xstrprintf ("%s %s", cmd, args));
+ run = xstrprintf ("%s %s", cmd, args);
else
run.reset (xstrdup (cmd));
execute_command ( /*ui */ run.get (), 0 /*from_tty */ );
diff --git a/gdb/python/py-breakpoint.c b/gdb/python/py-breakpoint.c
index d99d9b18b49..1b414a1e028 100644
--- a/gdb/python/py-breakpoint.c
+++ b/gdb/python/py-breakpoint.c
@@ -781,7 +781,7 @@ bppy_init (PyObject *self, PyObject *args, PyObject *kwargs)
if (lineobj != NULL)
{
if (PyInt_Check (lineobj))
- line.reset (xstrprintf ("%ld", PyInt_AsLong (lineobj)));
+ line = xstrprintf ("%ld", PyInt_AsLong (lineobj));
else if (PyString_Check (lineobj))
line = python_string_to_host_string (lineobj);
else
diff --git a/gdb/remote.c b/gdb/remote.c
index abf63de622a..c3601972400 100644
--- a/gdb/remote.c
+++ b/gdb/remote.c
@@ -1919,41 +1919,36 @@ static void
add_packet_config_cmd (struct packet_config *config, const char *name,
const char *title, int legacy)
{
- char *set_doc;
- char *show_doc;
- char *cmd_name;
-
config->name = name;
config->title = title;
- set_doc = xstrprintf ("Set use of remote protocol `%s' (%s) packet.",
- name, title);
- show_doc = xstrprintf ("Show current use of remote "
- "protocol `%s' (%s) packet.",
- name, title);
+ gdb::unique_xmalloc_ptr<char> set_doc
+ = xstrprintf ("Set use of remote protocol `%s' (%s) packet.",
+ name, title);
+ gdb::unique_xmalloc_ptr<char> show_doc
+ = xstrprintf ("Show current use of remote protocol `%s' (%s) packet.",
+ name, title);
/* set/show TITLE-packet {auto,on,off} */
- cmd_name = xstrprintf ("%s-packet", title);
+ gdb::unique_xmalloc_ptr<char> cmd_name = xstrprintf ("%s-packet", title);
set_show_commands cmds
- = add_setshow_auto_boolean_cmd (cmd_name, class_obscure,
- &config->detect, set_doc,
- show_doc, NULL, /* help_doc */
+ = add_setshow_auto_boolean_cmd (cmd_name.release (), class_obscure,
+ &config->detect, set_doc.get (),
+ show_doc.get (), NULL, /* help_doc */
NULL,
show_remote_protocol_packet_cmd,
&remote_set_cmdlist, &remote_show_cmdlist);
config->show_cmd = cmds.show;
- /* The command code copies the documentation strings. */
- xfree (set_doc);
- xfree (show_doc);
-
/* set/show remote NAME-packet {auto,on,off} -- legacy. */
if (legacy)
{
- char *legacy_name;
-
- legacy_name = xstrprintf ("%s-packet", name);
- add_alias_cmd (legacy_name, cmds.set, class_obscure, 0,
+ /* It's not clear who should take ownership of this string, so, for
+ now, make it static, and give copies to each of the add_alias_cmd
+ calls below. */
+ static gdb::unique_xmalloc_ptr<char> legacy_name
+ = xstrprintf ("%s-packet", name);
+ add_alias_cmd (legacy_name.get (), cmds.set, class_obscure, 0,
&remote_set_cmdlist);
- add_alias_cmd (legacy_name, cmds.show, class_obscure, 0,
+ add_alias_cmd (legacy_name.get (), cmds.show, class_obscure, 0,
&remote_show_cmdlist);
}
}
diff --git a/gdb/riscv-tdep.c b/gdb/riscv-tdep.c
index bfd93c65d22..29b864cdfcc 100644
--- a/gdb/riscv-tdep.c
+++ b/gdb/riscv-tdep.c
@@ -566,8 +566,8 @@ struct riscv_csr_feature : public riscv_register_feature
for (auto ® : m_registers)
{
int csr_num = reg.regnum - RISCV_FIRST_CSR_REGNUM;
- const char *alias = xstrprintf ("csr%d", csr_num);
- reg.names.push_back (alias);
+ gdb::unique_xmalloc_ptr<char> alias = xstrprintf ("csr%d", csr_num);
+ reg.names.push_back (alias.release ());
}
}
};
diff --git a/gdb/rust-lang.h b/gdb/rust-lang.h
index 6962f45d7b8..06de60ff7ea 100644
--- a/gdb/rust-lang.h
+++ b/gdb/rust-lang.h
@@ -113,9 +113,8 @@ class rust_language : public language_defn
{
type = check_typedef (TYPE_TARGET_TYPE (check_typedef (type)));
std::string name = type_to_string (type);
- return gdb::unique_xmalloc_ptr<char>
- (xstrprintf ("*(%s as *mut %s)", core_addr_to_string (addr),
- name.c_str ()));
+ return xstrprintf ("*(%s as *mut %s)", core_addr_to_string (addr),
+ name.c_str ());
}
/* See language.h. */
diff --git a/gdb/skip.c b/gdb/skip.c
index 4d24088f247..0498becbdf1 100644
--- a/gdb/skip.c
+++ b/gdb/skip.c
@@ -650,7 +650,7 @@ complete_skip_number (cmd_list_element *cmd,
for (const skiplist_entry &entry : skiplist_entries)
{
- gdb::unique_xmalloc_ptr<char> name (xstrprintf ("%d", entry.number ()));
+ gdb::unique_xmalloc_ptr<char> name = xstrprintf ("%d", entry.number ());
if (strncmp (word, name.get (), word_len) == 0)
completer.add_completion (std::move (name));
}
diff --git a/gdb/target.c b/gdb/target.c
index 6219393987e..8fe27c775ea 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -887,15 +887,15 @@ void
add_deprecated_target_alias (const target_info &tinfo, const char *alias)
{
struct cmd_list_element *c;
- char *alt;
/* If we use add_alias_cmd, here, we do not get the deprecated warning,
see PR cli/15104. */
c = add_cmd (alias, no_class, tinfo.doc, &targetlist);
c->func = open_target;
c->set_context ((void *) &tinfo);
- alt = xstrprintf ("target %s", tinfo.shortname);
- deprecate_cmd (c, alt);
+ gdb::unique_xmalloc_ptr<char> alt
+ = xstrprintf ("target %s", tinfo.shortname);
+ deprecate_cmd (c, alt.release ());
}
/* Stub functions */
diff --git a/gdb/tracepoint.c b/gdb/tracepoint.c
index 042fcc7f364..0c5107c8d5c 100644
--- a/gdb/tracepoint.c
+++ b/gdb/tracepoint.c
@@ -2809,7 +2809,7 @@ all_tracepoint_actions (struct breakpoint *t)
if (!default_collect.empty ())
{
gdb::unique_xmalloc_ptr<char> default_collect_line
- (xstrprintf ("collect %s", default_collect.c_str ()));
+ = xstrprintf ("collect %s", default_collect.c_str ());
validate_actionline (default_collect_line.get (), t);
actions.reset (new struct command_line (simple_control,
diff --git a/gdb/tui/tui-layout.c b/gdb/tui/tui-layout.c
index 2dfc51935d9..89f84a2b942 100644
--- a/gdb/tui/tui-layout.c
+++ b/gdb/tui/tui-layout.c
@@ -850,10 +850,10 @@ add_layout_command (const char *name, tui_layout_split *layout)
layout->specification (&spec, 0);
gdb::unique_xmalloc_ptr<char> doc
- (xstrprintf (_("Apply the \"%s\" layout.\n\
+ = xstrprintf (_("Apply the \"%s\" layout.\n\
This layout was created using:\n\
tui new-layout %s %s"),
- name, name, spec.c_str ()));
+ name, name, spec.c_str ());
cmd = add_cmd (name, class_tui, nullptr, doc.get (), &layout_list);
cmd->set_context (layout);
diff --git a/gdb/utils.c b/gdb/utils.c
index 92a847aef4d..e27a8818b94 100644
--- a/gdb/utils.c
+++ b/gdb/utils.c
@@ -534,10 +534,10 @@ add_internal_problem_command (struct internal_problem *problem)
set_doc and show_doc in this function. */
const char *set_doc
= xstrprintf (_("Configure what GDB does when %s is detected."),
- problem->name);
+ problem->name).release ();
const char *show_doc
= xstrprintf (_("Show what GDB does when %s is detected."),
- problem->name);
+ problem->name).release ();
add_setshow_prefix_cmd (problem->name, class_maintenance,
set_doc, show_doc, set_cmd_list, show_cmd_list,
diff --git a/gdb/xtensa-tdep.c b/gdb/xtensa-tdep.c
index 42bff4c818f..1f7eb59932a 100644
--- a/gdb/xtensa-tdep.c
+++ b/gdb/xtensa-tdep.c
@@ -720,7 +720,8 @@ xtensa_init_reggroups (void)
xtensa_vectra_reggroup = reggroup_new ("vectra", USER_REGGROUP);
for (i = 0; i < XTENSA_MAX_COPROCESSOR; i++)
- xtensa_cp[i] = reggroup_new (xstrprintf ("cp%d", i), USER_REGGROUP);
+ xtensa_cp[i] = reggroup_new (xstrprintf ("cp%d", i).release (),
+ USER_REGGROUP);
}
static void
diff --git a/gdbsupport/common-utils.cc b/gdbsupport/common-utils.cc
index 7a8434538dc..42bce36e535 100644
--- a/gdbsupport/common-utils.cc
+++ b/gdbsupport/common-utils.cc
@@ -32,19 +32,18 @@ xzalloc (size_t size)
/* Like asprintf/vasprintf but get an internal_error if the call
fails. */
-char *
+gdb::unique_xmalloc_ptr<char>
xstrprintf (const char *format, ...)
{
- char *ret;
va_list args;
va_start (args, format);
- ret = xstrvprintf (format, args);
+ gdb::unique_xmalloc_ptr<char> ret = xstrvprintf (format, args);
va_end (args);
return ret;
}
-char *
+gdb::unique_xmalloc_ptr<char>
xstrvprintf (const char *format, va_list ap)
{
char *ret = NULL;
@@ -56,7 +55,7 @@ xstrvprintf (const char *format, va_list ap)
happen, but just to be sure. */
if (ret == NULL || status < 0)
internal_error (__FILE__, __LINE__, _("vasprintf call failed"));
- return ret;
+ return gdb::unique_xmalloc_ptr<char> (ret);
}
int
diff --git a/gdbsupport/common-utils.h b/gdbsupport/common-utils.h
index 4a75e67079f..1e90a5c078b 100644
--- a/gdbsupport/common-utils.h
+++ b/gdbsupport/common-utils.h
@@ -23,6 +23,7 @@
#include <string>
#include <vector>
#include "gdbsupport/byte-vector.h"
+#include "gdbsupport/gdb_unique_ptr.h"
#include "poison.h"
@@ -54,8 +55,9 @@ void *xzalloc (size_t);
/* Like asprintf and vasprintf, but return the string, throw an error
if no memory. */
-char *xstrprintf (const char *format, ...) ATTRIBUTE_PRINTF (1, 2);
-char *xstrvprintf (const char *format, va_list ap)
+gdb::unique_xmalloc_ptr<char> xstrprintf (const char *format, ...)
+ ATTRIBUTE_PRINTF (1, 2);
+gdb::unique_xmalloc_ptr<char> xstrvprintf (const char *format, va_list ap)
ATTRIBUTE_PRINTF (1, 0);
/* Like snprintf, but throw an error if the output buffer is too small. */
--
2.25.4
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH 0/2] make xstrprintf and xstrvprintf return a unique_ptr
2021-11-08 17:11 [PATCH 0/2] make xstrprintf and xstrvprintf return a unique_ptr Andrew Burgess
2021-11-08 17:11 ` [PATCH 1/2] gdbsupport: move xfree into its own file Andrew Burgess
2021-11-08 17:11 ` [PATCH 2/2] gdb/gdbsupport: make xstrprintf and xstrvprintf return a unique_ptr Andrew Burgess
@ 2021-11-08 18:11 ` Tom Tromey
2 siblings, 0 replies; 4+ messages in thread
From: Tom Tromey @ 2021-11-08 18:11 UTC (permalink / raw)
To: Andrew Burgess via Gdb-patches; +Cc: Andrew Burgess
>>>>> "Andrew" == Andrew Burgess via Gdb-patches <gdb-patches@sourceware.org> writes:
Andrew> I thought it might be a good idea if xstrprintf return a
Andrew> gdb::unique_xmalloc_ptr<char>. It removes the need to manually call
Andrew> free in a few places and avoids us having to repack the char* into a
Andrew> unique_xmalloc_ptr in a few more places.
Andrew> There are plenty of places where I just end up calling '.release()' on
Andrew> the result, but I prefer that as it makes it explicit that the
Andrew> allocated pointer is "escaping". I'd hope, over time, we'd slowly
Andrew> remove these cases from GDB and use more managed pointers.
Andrew> Thoughts?
I agree this is a good direction, and both of these patches look ok to
me.
FWIW, every once in a while I look for uses of 'release' to 'xfree' and
try to remove some.
Tom
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-11-08 18:58 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-08 17:11 [PATCH 0/2] make xstrprintf and xstrvprintf return a unique_ptr Andrew Burgess
2021-11-08 17:11 ` [PATCH 1/2] gdbsupport: move xfree into its own file Andrew Burgess
2021-11-08 17:11 ` [PATCH 2/2] gdb/gdbsupport: make xstrprintf and xstrvprintf return a unique_ptr Andrew Burgess
2021-11-08 18:11 ` [PATCH 0/2] " Tom Tromey
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).