[Lldb-commits] [PATCH] D59015: [lldb-mi] Include full path in the -data-disassemble response
Anton Kolesov via Phabricator via lldb-commits
lldb-commits at lists.llvm.org
Fri Apr 26 06:04:27 PDT 2019
anton.kolesov added inline comments.
Comment at: lldb/tools/lldb-mi/MICmdCmdData.cpp:419
+ // Get a full path to the file.
+ std::unique_ptr<char> pPathBuffer(new char[PATH_MAX]);
+ lineEntry.GetFileSpec().GetPath(pPathBuffer.get(), PATH_MAX);
> anton.kolesov wrote:
> > clayborg wrote:
> > > Confused as to why we are calling malloc and free here for pPathBuffer? Why not just:
> > > ```
> > > char pPathBuffer[PATH_MAX];
> > > ```
> > I don't have a strong opinion on this, so to maintain consistency in the code I'm trying to use what other code in lldb-mi uses in similar situations - which is either unique_ptr or static local variable, but I presume it was decided that second approach is not good. FWIW, If I were to write code without regard of what is being done in the same project, then I would never ever had a variable named "miValueConst5".
> I would just use a local variable on the stack as I suggested. static variables are not correct and should never be used in cases like this, so if you see other errors, might be a good idea to submit a patch and improve lldb-mi. I am all for consistency where it makes sense, but I am also for fixing issues when we see them.
But what is wrong with the approach of using std::unique_ptr<char> local variable?
CHANGES SINCE LAST ACTION
More information about the lldb-commits