Fix memory leak in cp-support.c (cp_canonicalize_string)
Commit Message
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
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
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.
>
@@ -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.
@@ -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 ();