clayborg 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:
> > 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?
That forces a call to malloc(...) and free(...). No need to involve allocations 
on the heap for such simple things


Repository:
  rLLDB LLDB

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

https://reviews.llvm.org/D59015



_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to