[Lldb-commits] [PATCH] D100800: [lldb] Fix demangler leaks in the DWARF AST parser

[Fangrui Song via Phabricator via lldb-commits]

MaskRay added inline comments.


================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:1229
       if (!function_decl) {
+        char *buf = nullptr;
         llvm::StringRef name = attrs.name.GetStringRef();
----------------
teemperor wrote:
> `name_buf` ? From what I can see this could also be a unique_ptr with a custom deleter so that using `name` is always safe?
LG. Will update to use `name_buf`


================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:1229
       if (!function_decl) {
+        char *buf = nullptr;
         llvm::StringRef name = attrs.name.GetStringRef();
----------------
MaskRay wrote:
> teemperor wrote:
> > `name_buf` ? From what I can see this could also be a unique_ptr with a custom deleter so that using `name` is always safe?
> LG. Will update to use `name_buf`
A custom deleter is too inconvenient...


================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp:1249
             attrs.is_inline);
+        free(buf);
 
----------------
teemperor wrote:
> `std::free` ?
`std::` for C library functions is uncommon.

For some common functions (free,strcpy,memset,memcpy,...), the unqualified version is more common. I can find some `::foo` as well but `std::foo` is really rare.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100800