amccarth added inline comments.

================
Comment at: lldb/trunk/source/Host/common/FileSpec.cpp:1179
@@ +1178,3 @@
+
+            char child_path[PATH_MAX];
+            const int child_path_len = ::snprintf (child_path, 
sizeof(child_path), "%s\\%s", dir_path, fileName.c_str());
----------------
cameron314 wrote:
> amccarth wrote:
> > cameron314 wrote:
> > > amccarth wrote:
> > > > MAX_PATH in Windows.  (And someday, we should get rid of these fixed 
> > > > buffers so we can use UNC paths.)
> > > I absolutely agree re the fixed buffers. I'll try to make this change in 
> > > all of the functions that can allocate dynamic memory.
> > > 
> > > `MAX_PATH` is not `PATH_MAX`, though -- `MAX_PATH` is a Win32 constant of 
> > > 260, whereas `PATH_MAX` is an LLDB constant of 32K. (Actually PATH_MAX is 
> > > defined in multiple places and was either that or MAX_PATH depending on 
> > > which header was included, but mostly it was 32K so I changed them all to 
> > > at least be consistently 32K.)
> > Ah, I see.  I work in another code base where PATH_MAX is synonymous with 
> > MAX_PATH, so I was confused.
> > 
> > Buffers of 32K characters on the stack seem like a bad idea.  We need a 
> > vector or string or some other container that puts the bulk of the data 
> > somewhere other than the stack.
> Right. There's currently ~110 places in LLDB that allocate buffers of size 
> `PATH_MAX` on the stack. Nearly all of those predate my patch, it seems.
Right, but this is a new case because you changed it from MAX_PATH (260) to 
PATH_MAX (32K).  I'd rather not introduce more instances like this.


Repository:
  rL LLVM

http://reviews.llvm.org/D17107



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

Reply via email to