[PATCH 0/2] More uses of make_unique_xstrdup

[PATCH 0/2] More uses of make_unique_xstrdup

[Tom Tromey]

This replaces some uses of '.reset(xstrdup())' with
'= make_unique_xstrdup()'.  I think explicit uses of reset
should be avoided when possible.

Regression tested on x86-64 Fedora 41.

Signed-off-by: Tom Tromey <tromey@adacore.com>
---
Tom Tromey (2):
      Use make_unique_xstrdup in more places
      Use make_unique_xstrdup in tracepoint

 gdb/breakpoint.c            |  4 ++--
 gdb/buildsym.h              |  8 +++++---
 gdb/cli/cli-dump.c          |  2 +-
 gdb/mi/mi-cmd-env.c         |  2 +-
 gdb/python/py-framefilter.c |  2 +-
 gdb/python/py-function.c    |  2 +-
 gdb/python/py-lazy-string.c |  5 ++++-
 gdb/python/py-param.c       |  2 +-
 gdb/solib.c                 |  4 ++--
 gdb/source.c                |  4 ++--
 gdb/stack.c                 | 16 +++++++++++-----
 gdb/tracectf.c              |  4 ++--
 gdb/tracepoint.c            | 10 ++++------
 gdb/tracepoint.h            | 12 ++++++------
 14 files changed, 43 insertions(+), 34 deletions(-)
---
base-commit: 44c39324673cd9bb18db67234253f288497d66a1
change-id: 20260115-xstrdup-00eb39b61e16

Best regards,
-- 
Tom Tromey <tromey@adacore.com>

[PATCH 1/2] Use make_unique_xstrdup in more places

[Tom Tromey]

This replaces a number of uses of 'ptr.reset (xstrdup ())'
with 'ptr = make_unique_xstrdup ()'.

The main motivation for this is that, IMO, it's better to avoid the
reset method when possible.
---
 gdb/breakpoint.c            |  4 ++--
 gdb/buildsym.h              |  8 +++++---
 gdb/cli/cli-dump.c          |  2 +-
 gdb/mi/mi-cmd-env.c         |  2 +-
 gdb/python/py-framefilter.c |  2 +-
 gdb/python/py-function.c    |  2 +-
 gdb/python/py-lazy-string.c |  5 ++++-
 gdb/python/py-param.c       |  2 +-
 gdb/solib.c                 |  4 ++--
 gdb/source.c                |  4 ++--
 gdb/stack.c                 | 16 +++++++++++-----
 gdb/tracectf.c              |  4 ++--
 12 files changed, 33 insertions(+), 22 deletions(-)

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index af4de248ab6..a4ccad32a8b 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -9296,9 +9296,9 @@ create_breakpoint (struct gdbarch *gdbarch,
   else
     {
       if (cond_string != nullptr)
-	cond_string_copy.reset (xstrdup (cond_string));
+	cond_string_copy = make_unique_xstrdup (cond_string);
       if (extra_string != nullptr)
-	extra_string_copy.reset (xstrdup (extra_string));
+	extra_string_copy = make_unique_xstrdup (extra_string);
     }
 
   /* Clear these.  Updated values are now held in the *_copy locals.  */
diff --git a/gdb/buildsym.h b/gdb/buildsym.h
index 7ca5e57c7f4..eca248df79e 100644
--- a/gdb/buildsym.h
+++ b/gdb/buildsym.h
@@ -152,7 +152,7 @@ struct buildsym_compunit
 		     const char *comp_dir_, enum language language_,
 		     CORE_ADDR last_addr, struct compunit_symtab *cust)
     : m_objfile (objfile_),
-      m_last_source_file (name == nullptr ? nullptr : xstrdup (name)),
+      m_last_source_file (name == nullptr ? nullptr : make_unique_xstrdup (name)),
       m_comp_dir (comp_dir_ == nullptr ? "" : comp_dir_),
       m_compunit_symtab (cust),
       m_language (language_),
@@ -166,8 +166,10 @@ struct buildsym_compunit
 
   void set_last_source_file (const char *name)
   {
-    char *new_name = name == NULL ? NULL : xstrdup (name);
-    m_last_source_file.reset (new_name);
+    if (name == nullptr)
+      m_last_source_file = nullptr;
+    else
+      m_last_source_file = make_unique_xstrdup (name);
   }
 
   const char *get_last_source_file ()
diff --git a/gdb/cli/cli-dump.c b/gdb/cli/cli-dump.c
index 16559a72068..3084aa11128 100644
--- a/gdb/cli/cli-dump.c
+++ b/gdb/cli/cli-dump.c
@@ -62,7 +62,7 @@ scan_filename (const char **cmd, const char *defname)
     {
       if (defname == NULL)
 	error (_("Missing filename."));
-      filename.reset (xstrdup (defname));
+      filename = make_unique_xstrdup (defname);
     }
   else
     {
diff --git a/gdb/mi/mi-cmd-env.c b/gdb/mi/mi-cmd-env.c
index fdfa3b002cf..8fb952f8ee1 100644
--- a/gdb/mi/mi-cmd-env.c
+++ b/gdb/mi/mi-cmd-env.c
@@ -47,7 +47,7 @@ env_execute_cli_command (const char *cmd, const char *args)
       if (args != NULL)
 	run = xstrprintf ("%s %s", cmd, args);
       else
-	run.reset (xstrdup (cmd));
+	run = make_unique_xstrdup (cmd);
       execute_command ( /*ui */ run.get (), 0 /*from_tty */ );
     }
 }
diff --git a/gdb/python/py-framefilter.c b/gdb/python/py-framefilter.c
index d26b0a4fc99..7e54286abbc 100644
--- a/gdb/python/py-framefilter.c
+++ b/gdb/python/py-framefilter.c
@@ -102,7 +102,7 @@ extract_sym (PyObject *obj, gdb::unique_xmalloc_ptr<char> *name,
 
       /* Duplicate the symbol name, so the caller has consistency
 	 in garbage collection.  */
-      name->reset (xstrdup ((*sym)->print_name ()));
+      *name = make_unique_xstrdup ((*sym)->print_name ());
 
       /* If a symbol is specified attempt to determine the language
 	 from the symbol.  If mode is not "auto", then the language
diff --git a/gdb/python/py-function.c b/gdb/python/py-function.c
index 7f072bcee0d..ee60f08b65c 100644
--- a/gdb/python/py-function.c
+++ b/gdb/python/py-function.c
@@ -125,7 +125,7 @@ fnpy_init (PyObject *self, PyObject *args, PyObject *kwds)
 	}
     }
   if (! docstring)
-    docstring.reset (xstrdup (_("This function is not documented.")));
+    docstring = make_unique_xstrdup (_("This function is not documented."));
 
   add_internal_function (make_unique_xstrdup (name), std::move (docstring),
 			 fnpy_call, self_ref.release ());
diff --git a/gdb/python/py-lazy-string.c b/gdb/python/py-lazy-string.c
index caa42e5f0c0..41f958744c1 100644
--- a/gdb/python/py-lazy-string.c
+++ b/gdb/python/py-lazy-string.c
@@ -294,7 +294,10 @@ gdbpy_extract_lazy_string (PyObject *string, CORE_ADDR *addr,
   *addr = lazy->address;
   *str_elt_type = stpy_lazy_string_elt_type (lazy);
   *length = lazy->length;
-  encoding->reset (lazy->encoding ? xstrdup (lazy->encoding) : NULL);
+  if (lazy->encoding == nullptr)
+    *encoding = nullptr;
+  else
+    *encoding = make_unique_xstrdup (lazy->encoding);
 }
 
 /* __str__ for LazyString.  */
diff --git a/gdb/python/py-param.c b/gdb/python/py-param.c
index 09cd3b9e390..f305661fa43 100644
--- a/gdb/python/py-param.c
+++ b/gdb/python/py-param.c
@@ -501,7 +501,7 @@ get_doc_string (PyObject *object, enum doc_string_type doc_type,
       || (doc_type != doc_string_description && *result == '\0'))
     {
       if (doc_type == doc_string_description)
-	result.reset (xstrdup (_("This command is not documented.")));
+	result = make_unique_xstrdup (_("This command is not documented."));
       else
 	{
 	  if (doc_type == doc_string_show)
diff --git a/gdb/solib.c b/gdb/solib.c
index 492ff5580c9..06330de7e91 100644
--- a/gdb/solib.c
+++ b/gdb/solib.c
@@ -174,7 +174,7 @@ solib_find_1 (const char *in_pathname, int *fd, bool is_solib)
   */
 
   if (!IS_TARGET_ABSOLUTE_PATH (fskind, in_pathname) || sysroot == NULL)
-    temp_pathname.reset (xstrdup (in_pathname));
+    temp_pathname = make_unique_xstrdup (in_pathname);
   else
     {
       bool need_dir_separator;
@@ -357,7 +357,7 @@ exec_file_find (const char *in_pathname, int *fd)
 	 filename.  Not much more we can do...)  */
 
       if (!source_full_path_of (in_pathname, &result))
-	result.reset (xstrdup (in_pathname));
+	result = make_unique_xstrdup (in_pathname);
       if (fd != NULL)
 	*fd = -1;
     }
diff --git a/gdb/source.c b/gdb/source.c
index 84f3fdc8a67..ade1bb2789f 100644
--- a/gdb/source.c
+++ b/gdb/source.c
@@ -1180,7 +1180,7 @@ find_source_or_rewrite (const char *filename, const char *dirname)
 	 should report the pathname where GDB tried to find the file.  */
 
       if (dirname == nullptr || IS_ABSOLUTE_PATH (filename))
-	fullname.reset (xstrdup (filename));
+	fullname = make_unique_xstrdup (filename);
       else
 	fullname.reset (concat (dirname, SLASH_STRING,
 				filename, (char *) nullptr));
@@ -1222,7 +1222,7 @@ symtab_to_fullname (struct symtab *s)
 
 	  if (s->compunit ()->dirname () == nullptr
 	      || IS_ABSOLUTE_PATH (s->filename ()))
-	    fullname.reset (xstrdup (s->filename ()));
+	    fullname = make_unique_xstrdup (s->filename ());
 	  else
 	    fullname.reset (concat (s->compunit ()->dirname (), SLASH_STRING,
 				    s->filename (), (char *) NULL));
diff --git a/gdb/stack.c b/gdb/stack.c
index 9d8e9da6aa8..a0abf4cda18 100644
--- a/gdb/stack.c
+++ b/gdb/stack.c
@@ -511,7 +511,7 @@ read_frame_local (struct symbol *sym, const frame_info_ptr &frame,
     }
   catch (const gdb_exception_error &except)
     {
-      argp->error.reset (xstrdup (except.what ()));
+      argp->error = make_unique_xstrdup (except.what ());
     }
 }
 
@@ -682,7 +682,10 @@ read_frame_arg (const frame_print_options &fp_opts,
 
   argp->sym = sym;
   argp->val = val;
-  argp->error.reset (val_error ? xstrdup (val_error) : NULL);
+  if (val_error == nullptr)
+    argp->error = nullptr;
+  else
+    argp->error = make_unique_xstrdup (val_error);
   if (!val && !val_error)
     argp->entry_kind = print_entry_values_only;
   else if ((fp_opts.print_entry_values == print_entry_values_compact
@@ -697,7 +700,10 @@ read_frame_arg (const frame_print_options &fp_opts,
 
   entryargp->sym = sym;
   entryargp->val = entryval;
-  entryargp->error.reset (entryval_error ? xstrdup (entryval_error) : NULL);
+  if (entryval_error == nullptr)
+    entryargp->error = nullptr;
+  else
+    entryargp->error = make_unique_xstrdup (entryval_error);
   if (!entryval && !entryval_error)
     entryargp->entry_kind = print_entry_values_no;
   else
@@ -1294,7 +1300,7 @@ find_frame_funname (const frame_info_ptr &frame, enum language *funlang,
       /* If we didn't hit the C++ case above, set *funname
 	 here.  */
       if (funname == NULL)
-	funname.reset (xstrdup (print_name));
+	funname = make_unique_xstrdup (print_name);
     }
   else
     {
@@ -1306,7 +1312,7 @@ find_frame_funname (const frame_info_ptr &frame, enum language *funlang,
       bound_minimal_symbol msymbol = lookup_minimal_symbol_by_pc (pc);
       if (msymbol.minsym != NULL)
 	{
-	  funname.reset (xstrdup (msymbol.minsym->print_name ()));
+	  funname = make_unique_xstrdup (msymbol.minsym->print_name ());
 	  *funlang = msymbol.minsym->language ();
 	}
     }
diff --git a/gdb/tracectf.c b/gdb/tracectf.c
index 9fcb56bf2ff..3c6bf6cb5a9 100644
--- a/gdb/tracectf.c
+++ b/gdb/tracectf.c
@@ -1042,9 +1042,9 @@ ctf_read_tsv (struct uploaded_tsv **uploaded_tsvs)
 							   #FIELD));	\
 									\
       if (strlen (p) > 0)						\
-	(VAR)->FIELD.reset (xstrdup (p));				\
+	(VAR)->FIELD = make_unique_xstrdup (p);				\
       else								\
-	(VAR)->FIELD = NULL;						\
+	(VAR)->FIELD = nullptr;						\
     }									\
   while (0)
 

-- 
2.52.0

[PATCH 2/2] Use make_unique_xstrdup in tracepoint

[Tom Tromey]

This changes the tracepoint code to use make_unique_xstrdup (and
make_unique_xstrndup).  This meant changing the types of some members
of uploaded_tp -- but it seems to me that using array types there did
not add any value.
---
 gdb/tracepoint.c | 10 ++++------
 gdb/tracepoint.h | 12 ++++++------
 2 files changed, 10 insertions(+), 12 deletions(-)

diff --git a/gdb/tracepoint.c b/gdb/tracepoint.c
index a70edaeb262..2b6fbcbf5ae 100644
--- a/gdb/tracepoint.c
+++ b/gdb/tracepoint.c
@@ -3414,7 +3414,7 @@ parse_tracepoint_definition (const char *line, struct uploaded_tp **utpp)
   p++;  /* skip a colon */
   if (piece == 'T')
     {
-      gdb::unique_xmalloc_ptr<char[]> cond;
+      gdb::unique_xmalloc_ptr<char> cond;
 
       enabled = (*p++ == 'E');
       p++;  /* skip a colon */
@@ -3442,9 +3442,7 @@ parse_tracepoint_definition (const char *line, struct uploaded_tp **utpp)
 	      p++;
 	      p = unpack_varlen_hex (p, &xlen);
 	      p++;  /* skip a comma */
-	      cond.reset ((char *) xmalloc (2 * xlen + 1));
-	      strncpy (&cond[0], p, 2 * xlen);
-	      cond[2 * xlen] = '\0';
+	      cond = make_unique_xstrndup (p, 2 * xlen);
 	      p += 2 * xlen;
 	    }
 	  else
@@ -3486,9 +3484,9 @@ parse_tracepoint_definition (const char *line, struct uploaded_tp **utpp)
       buf[end] = '\0';
 
       if (startswith (srctype, "at:"))
-	utp->at_string.reset (xstrdup (buf));
+	utp->at_string = make_unique_xstrdup (buf);
       else if (startswith (srctype, "cond:"))
-	utp->cond_string.reset (xstrdup (buf));
+	utp->cond_string = make_unique_xstrdup (buf);
       else if (startswith (srctype, "cmd:"))
 	utp->cmd_strings.emplace_back (xstrdup (buf));
     }
diff --git a/gdb/tracepoint.h b/gdb/tracepoint.h
index 8503789a14b..247723aa7ee 100644
--- a/gdb/tracepoint.h
+++ b/gdb/tracepoint.h
@@ -177,21 +177,21 @@ struct uploaded_tp
   int orig_size = 0;
 
   /* String that is the encoded form of the tracepoint's condition.  */
-  gdb::unique_xmalloc_ptr<char[]> cond;
+  gdb::unique_xmalloc_ptr<char> cond;
 
   /* Vectors of strings that are the encoded forms of a tracepoint's
      actions.  */
-  std::vector<gdb::unique_xmalloc_ptr<char[]>> actions;
-  std::vector<gdb::unique_xmalloc_ptr<char[]>> step_actions;
+  std::vector<gdb::unique_xmalloc_ptr<char>> actions;
+  std::vector<gdb::unique_xmalloc_ptr<char>> step_actions;
 
   /* The original string defining the location of the tracepoint.  */
-  gdb::unique_xmalloc_ptr<char[]> at_string;
+  gdb::unique_xmalloc_ptr<char> at_string;
 
   /* The original string defining the tracepoint's condition.  */
-  gdb::unique_xmalloc_ptr<char[]> cond_string;
+  gdb::unique_xmalloc_ptr<char> cond_string;
 
   /* List of original strings defining the tracepoint's actions.  */
-  std::vector<gdb::unique_xmalloc_ptr<char[]>> cmd_strings;
+  std::vector<gdb::unique_xmalloc_ptr<char>> cmd_strings;
 
   /* The tracepoint's current hit count.  */
   int hit_count = 0;

-- 
2.52.0

Re: [PATCH 0/2] More uses of make_unique_xstrdup

[Guinevere Larsen]

On 1/15/26 2:33 PM, Tom Tromey wrote:
> This replaces some uses of '.reset(xstrdup())' with
> '= make_unique_xstrdup()'.  I think explicit uses of reset
> should be avoided when possible.
>
> Regression tested on x86-64 Fedora 41.
>
> Signed-off-by: Tom Tromey <tromey@adacore.com>

This makes me wonder... is there any reason to prefer xmalloc/xstrdup 
instead of using std::string?

My preference is to use stdlib stuff when possible, but maybe I'm 
misjudging the size of the refactor or there's a reason to prefer the 
current memory strategy.

-- 
Cheers,
Guinevere Larsen
It/she

> ---
> Tom Tromey (2):
>        Use make_unique_xstrdup in more places
>        Use make_unique_xstrdup in tracepoint
>
>   gdb/breakpoint.c            |  4 ++--
>   gdb/buildsym.h              |  8 +++++---
>   gdb/cli/cli-dump.c          |  2 +-
>   gdb/mi/mi-cmd-env.c         |  2 +-
>   gdb/python/py-framefilter.c |  2 +-
>   gdb/python/py-function.c    |  2 +-
>   gdb/python/py-lazy-string.c |  5 ++++-
>   gdb/python/py-param.c       |  2 +-
>   gdb/solib.c                 |  4 ++--
>   gdb/source.c                |  4 ++--
>   gdb/stack.c                 | 16 +++++++++++-----
>   gdb/tracectf.c              |  4 ++--
>   gdb/tracepoint.c            | 10 ++++------
>   gdb/tracepoint.h            | 12 ++++++------
>   14 files changed, 43 insertions(+), 34 deletions(-)
> ---
> base-commit: 44c39324673cd9bb18db67234253f288497d66a1
> change-id: 20260115-xstrdup-00eb39b61e16
>
> Best regards,

Re: [PATCH 0/2] More uses of make_unique_xstrdup

[Simon Marchi]

On 1/15/26 12:39 PM, Guinevere Larsen wrote:
> On 1/15/26 2:33 PM, Tom Tromey wrote:
>> This replaces some uses of '.reset(xstrdup())' with
>> '= make_unique_xstrdup()'.  I think explicit uses of reset
>> should be avoided when possible.
>>
>> Regression tested on x86-64 Fedora 41.
>>
>> Signed-off-by: Tom Tromey <tromey@adacore.com>
> 
> This makes me wonder... is there any reason to prefer xmalloc/xstrdup instead of using std::string?
> 
> My preference is to use stdlib stuff when possible, but maybe I'm misjudging the size of the refactor or there's a reason to prefer the current memory strategy.

When practical to switch to std::string, it typically is an improvement.
It makes strings manipulation safer and clearer.  There might be cases
where it is not trivial to use std::string immediately though, so
switching to make_unique_xstrdup is a good incremental change
nonetheless.

Simon

Re: [PATCH 0/2] More uses of make_unique_xstrdup

[Guinevere Larsen]

On 1/15/26 3:01 PM, Simon Marchi wrote:
> On 1/15/26 12:39 PM, Guinevere Larsen wrote:
>> On 1/15/26 2:33 PM, Tom Tromey wrote:
>>> This replaces some uses of '.reset(xstrdup())' with
>>> '= make_unique_xstrdup()'.  I think explicit uses of reset
>>> should be avoided when possible.
>>>
>>> Regression tested on x86-64 Fedora 41.
>>>
>>> Signed-off-by: Tom Tromey <tromey@adacore.com>
>> This makes me wonder... is there any reason to prefer xmalloc/xstrdup instead of using std::string?
>>
>> My preference is to use stdlib stuff when possible, but maybe I'm misjudging the size of the refactor or there's a reason to prefer the current memory strategy.
> When practical to switch to std::string, it typically is an improvement.
> It makes strings manipulation safer and clearer.  There might be cases
> where it is not trivial to use std::string immediately though, so
> switching to make_unique_xstrdup is a good incremental change
> nonetheless.
>
> Simon
>
Right that makes sense. I was moreso double-checking my gut feeling, 
rather than asserting that the series should convert to std::string

-- 
Cheers,
Guinevere Larsen
It/she

Re: [PATCH 0/2] More uses of make_unique_xstrdup

[Andrew Burgess]

Tom Tromey <tromey@adacore.com> writes:

> This replaces some uses of '.reset(xstrdup())' with
> '= make_unique_xstrdup()'.  I think explicit uses of reset
> should be avoided when possible.
>
> Regression tested on x86-64 Fedora 41.

All LGTM.

Approved-By: Andrew Burgess <aburgess@redhat.com>

Thanks,
Andrew


>
> Signed-off-by: Tom Tromey <tromey@adacore.com>
> ---
> Tom Tromey (2):
>       Use make_unique_xstrdup in more places
>       Use make_unique_xstrdup in tracepoint
>
>  gdb/breakpoint.c            |  4 ++--
>  gdb/buildsym.h              |  8 +++++---
>  gdb/cli/cli-dump.c          |  2 +-
>  gdb/mi/mi-cmd-env.c         |  2 +-
>  gdb/python/py-framefilter.c |  2 +-
>  gdb/python/py-function.c    |  2 +-
>  gdb/python/py-lazy-string.c |  5 ++++-
>  gdb/python/py-param.c       |  2 +-
>  gdb/solib.c                 |  4 ++--
>  gdb/source.c                |  4 ++--
>  gdb/stack.c                 | 16 +++++++++++-----
>  gdb/tracectf.c              |  4 ++--
>  gdb/tracepoint.c            | 10 ++++------
>  gdb/tracepoint.h            | 12 ++++++------
>  14 files changed, 43 insertions(+), 34 deletions(-)
> ---
> base-commit: 44c39324673cd9bb18db67234253f288497d66a1
> change-id: 20260115-xstrdup-00eb39b61e16
>
> Best regards,
> -- 
> Tom Tromey <tromey@adacore.com>