[1/5] gdbserver: make arch and osabi names gdb::unique_xmalloc_ptr<char>

Message ID 642bc25ebd1e94f3378e109256b1c1229fb5f624.1728407374.git.aburgess@redhat.com
State New
Headers
Series Set osabi in remote target descriptions |

Checks

Context Check Description
linaro-tcwg-bot/tcwg_gdb_build--master-aarch64 success Build passed
linaro-tcwg-bot/tcwg_gdb_build--master-arm success Build passed
linaro-tcwg-bot/tcwg_gdb_check--master-arm success Test passed
linaro-tcwg-bot/tcwg_gdb_check--master-aarch64 success Test passed

Commit Message

Andrew Burgess Oct. 8, 2024, 5:11 p.m. UTC
  Convert target_desc::arch and target_desc::osabi from 'const char*' to
gdb::unique_xmalloc_ptr<char>.  This also allows us to remove the user
defined ~target_desc destructor.

I doubt it ever actually occurred, but in theory at least, there was a
memory leak in set_tdesc_architecture and set_tdesc_osabi where the
member variables were assigned without freeing any previous
value... but I suspect that usually these fields are only set once.

There should be no user visible changes after this commit.

Approved-By: Tom Tromey <tom@tromey.com>
---
 gdbserver/tdesc.cc | 16 +++++-----------
 gdbserver/tdesc.h  |  6 ++----
 2 files changed, 7 insertions(+), 15 deletions(-)
  

Comments

Simon Marchi Oct. 10, 2024, 1:37 p.m. UTC | #1
> diff --git a/gdbserver/tdesc.h b/gdbserver/tdesc.h
> index 534b8b000f5..6fc37d038bc 100644
> --- a/gdbserver/tdesc.h
> +++ b/gdbserver/tdesc.h
> @@ -54,18 +54,16 @@ struct target_desc final : tdesc_element
>    mutable const char *xmltarget = NULL;
>  
>    /* The value of <architecture> element in the XML, replying GDB.  */
> -  const char *arch = NULL;
> +  gdb::unique_xmalloc_ptr<char> arch = nullptr;

Just a nit, you don't need to initialize unique_ptrs to nullptr, they
are default initialized to that.

Simon
  
Andrew Burgess Oct. 10, 2024, 3:31 p.m. UTC | #2
Simon Marchi <simark@simark.ca> writes:

>> diff --git a/gdbserver/tdesc.h b/gdbserver/tdesc.h
>> index 534b8b000f5..6fc37d038bc 100644
>> --- a/gdbserver/tdesc.h
>> +++ b/gdbserver/tdesc.h
>> @@ -54,18 +54,16 @@ struct target_desc final : tdesc_element
>>    mutable const char *xmltarget = NULL;
>>  
>>    /* The value of <architecture> element in the XML, replying GDB.  */
>> -  const char *arch = NULL;
>> +  gdb::unique_xmalloc_ptr<char> arch = nullptr;
>
> Just a nit, you don't need to initialize unique_ptrs to nullptr, they
> are default initialized to that.

Fixed.

Thanks,
Andrew
  

Patch

diff --git a/gdbserver/tdesc.cc b/gdbserver/tdesc.cc
index 3265793da96..7f11120c318 100644
--- a/gdbserver/tdesc.cc
+++ b/gdbserver/tdesc.cc
@@ -20,12 +20,6 @@ 
 
 #ifndef IN_PROCESS_AGENT
 
-target_desc::~target_desc ()
-{
-  xfree ((char *) arch);
-  xfree ((char *) osabi);
-}
-
 bool target_desc::operator== (const target_desc &other) const
 {
   if (reg_defs != other.reg_defs)
@@ -162,7 +156,7 @@  tdesc_compatible_info_arch_name (const tdesc_compatible_info_up &c_info)
 const char *
 tdesc_architecture_name (const struct target_desc *target_desc)
 {
-  return target_desc->arch;
+  return target_desc->arch.get ();
 }
 
 /* See gdbsupport/tdesc.h.  */
@@ -171,7 +165,7 @@  void
 set_tdesc_architecture (struct target_desc *target_desc,
 			const char *name)
 {
-  target_desc->arch = xstrdup (name);
+  target_desc->arch = make_unique_xstrdup (name);
 }
 
 /* See gdbsupport/tdesc.h.  */
@@ -179,7 +173,7 @@  set_tdesc_architecture (struct target_desc *target_desc,
 const char *
 tdesc_osabi_name (const struct target_desc *target_desc)
 {
-  return target_desc->osabi;
+  return target_desc->osabi.get ();
 }
 
 /* See gdbsupport/tdesc.h.  */
@@ -187,7 +181,7 @@  tdesc_osabi_name (const struct target_desc *target_desc)
 void
 set_tdesc_osabi (struct target_desc *target_desc, const char *name)
 {
-  target_desc->osabi = xstrdup (name);
+  target_desc->osabi = make_unique_xstrdup (name);
 }
 
 /* See gdbsupport/tdesc.h.  */
@@ -198,7 +192,7 @@  tdesc_get_features_xml (const target_desc *tdesc)
   /* Either .xmltarget or .features is not NULL.  */
   gdb_assert (tdesc->xmltarget != NULL
 	      || (!tdesc->features.empty ()
-		  && tdesc->arch != NULL));
+		  && tdesc_architecture_name (tdesc) != nullptr));
 
   if (tdesc->xmltarget == NULL)
     {
diff --git a/gdbserver/tdesc.h b/gdbserver/tdesc.h
index 534b8b000f5..6fc37d038bc 100644
--- a/gdbserver/tdesc.h
+++ b/gdbserver/tdesc.h
@@ -54,18 +54,16 @@  struct target_desc final : tdesc_element
   mutable const char *xmltarget = NULL;
 
   /* The value of <architecture> element in the XML, replying GDB.  */
-  const char *arch = NULL;
+  gdb::unique_xmalloc_ptr<char> arch = nullptr;
 
   /* The value of <osabi> element in the XML, replying GDB.  */
-  const char *osabi = NULL;
+  gdb::unique_xmalloc_ptr<char> osabi = nullptr;
 
 public:
   target_desc ()
     : registers_size (0)
   {}
 
-  ~target_desc ();
-
   bool operator== (const target_desc &other) const;
 
   bool operator!= (const target_desc &other) const