[Lldb-commits] [PATCH] D100795: [lldb] Fix RichManglingContext::FromCxxMethodName() leak

Jonas Devlieghere via Phabricator via lldb-commits lldb-commits at lists.llvm.org
Tue Apr 20 19:01:54 PDT 2021


JDevlieghere added inline comments.


================
Comment at: lldb/source/Core/RichManglingContext.cpp:23
+RichManglingContext::~RichManglingContext() {
+  std::free(m_ipd_buf);
+  ResetCxxMethodParser();
----------------
shafik wrote:
> rupprecht wrote:
> > JDevlieghere wrote:
> > > Instead of managing memory by hand, can we use a `unique_ptr<char[]>` instead?
> > The buffer here is created by `malloc`, and from a cursory reading of `processIPDStrResult`, can be passed to other methods that call `realloc` on it. It should be paired with `free`, not `delete`. You could put that in a `unique_ptr<char, FreeDeleter>`, but when you go down that road, I think it's probably simpler to leave as-is. (You'd also have to take care to always manually `.release()` it when updating it to the realloc'd value, because you don't want to delete the pre-realloc'd buffer).
> > 
> > (Note: this line is pulled from the original `~RichManglingContext()` definition in the header. I didn't write it so I can't defend it that well :) )
> I didn't suggest this b/c it was clear it was a quick fix and it seemed a reach to ask for that in this fix.
Thanks for the explanation! That does sound like a bit of overkill. If it's not already documented, would it be useful to leave that as a comment somewhere? 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100795/new/

https://reviews.llvm.org/D100795



More information about the lldb-commits mailing list