diff mbox

Fix memory leak in cp-support.c (cp_canonicalize_string)

Message ID 86k22dyno6.fsf@gmail.com
State New, archived
Headers

Commit Message

Yao Qi Aug. 9, 2017, 11:40 a.m. UTC 
  Alex Lindsay <alexlindsay239@gmail.com> writes:

> Formerly, in cp_canonicalize_string in cp-support.c, the return value of
> cp_comp_to_string was never freed, creating a sizable memory leak detectable
> with valgrind. This patch fixes the leak. However, a longer term solution
> would be to change the return type of cp_comp_to_string to
> gdb::unique_xmalloc_ptr<char>.

Hi Alex,
Thanks a lot for the investigation and the patch.  I revise it a little
to use gdb::unique_xmalloc_ptr<char>, and fix another leak somewhere else.
Patch below is pushed in.

Comments

Pedro Alves Aug. 9, 2017, 12:09 p.m. UTC | #1 
On 08/09/2017 12:40 PM, Yao Qi wrote:
> Alex Lindsay <alexlindsay239@gmail.com> writes:
> 
>> Formerly, in cp_canonicalize_string in cp-support.c, the return value of
>> cp_comp_to_string was never freed, creating a sizable memory leak detectable
>> with valgrind. This patch fixes the leak. However, a longer term solution
>> would be to change the return type of cp_comp_to_string to
>> gdb::unique_xmalloc_ptr<char>.
> 
> Hi Alex,
> Thanks a lot for the investigation and the patch.  I revise it a little
> to use gdb::unique_xmalloc_ptr<char>, and fix another leak somewhere else.
> Patch below is pushed in.
> 

Sorry about that, and thanks for the fix.

I think I'd be good if cp_comp_to_string was changed to
return a unique_ptr itself, to avoid similar cases creeping
back in.

As penance, I'll give it a try.

Thanks,
Pedro Alves
Alex Lindsay Aug. 9, 2017, 1:24 p.m. UTC | #2 
Thanks Yao!

On 08/09/2017 06:40 AM, Yao Qi wrote:
> Alex Lindsay <alexlindsay239@gmail.com> writes:
>
>> Formerly, in cp_canonicalize_string in cp-support.c, the return value of
>> cp_comp_to_string was never freed, creating a sizable memory leak detectable
>> with valgrind. This patch fixes the leak. However, a longer term solution
>> would be to change the return type of cp_comp_to_string to
>> gdb::unique_xmalloc_ptr<char>.
> Hi Alex,
> Thanks a lot for the investigation and the patch.  I revise it a little
> to use gdb::unique_xmalloc_ptr<char>, and fix another leak somewhere else.
> Patch below is pushed in.
>
diff mbox

Patch

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 97c39d7..209d0b6 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,10 @@ 
+2017-08-09  Alex Lindsay  <alexlindsay239@gmail.com>
+	    Yao Qi  <yao.qi@linaro.org>
+
+	* cp-support.c (cp_canonicalize_string_full): Use
+	gdb::unique_xmalloc_ptr<char>.
+	(cp_canonicalize_string): Likewise.
+
 2017-08-09  Yao Qi  <yao.qi@linaro.org>
 
 	* features/Makefile (WHICH): Remove i386/ non-linux stuff.
diff --git a/gdb/cp-support.c b/gdb/cp-support.c
index df9a563..f6557ab 100644
--- a/gdb/cp-support.c
+++ b/gdb/cp-support.c
@@ -527,9 +527,11 @@  cp_canonicalize_string_full (const char *string,
       replace_typedefs (info.get (), info->tree, finder, data);
 
       /* Convert the tree back into a string.  */
-      ret = cp_comp_to_string (info->tree, estimated_len);
-      gdb_assert (!ret.empty ());
+      gdb::unique_xmalloc_ptr<char> us (cp_comp_to_string (info->tree,
+							   estimated_len));
+      gdb_assert (us);
 
+      ret = us.get ();
       /* Finally, compare the original string with the computed
 	 name, returning NULL if they are the same.  */
       if (ret == string)
@@ -566,15 +568,18 @@  cp_canonicalize_string (const char *string)
     return std::string ();
 
   estimated_len = strlen (string) * 2;
-  std::string ret = cp_comp_to_string (info->tree, estimated_len);
+  gdb::unique_xmalloc_ptr<char> us (cp_comp_to_string (info->tree,
+						       estimated_len));
 
-  if (ret.empty ())
+  if (!us)
     {
       warning (_("internal error: string \"%s\" failed to be canonicalized"),
 	       string);
       return std::string ();
     }
 
+  std::string ret (us.get ());
+
   if (ret == string)
     return std::string ();