cameron314 added inline comments. ================ Comment at: lldb/trunk/source/Host/common/FileSpec.cpp:242 @@ -221,1 +241,3 @@ + path.push_back(0); + path.pop_back(); } ---------------- amccarth wrote: > I recognize that you're just repeating the pattern from above, but ... > > This seems whacky and dangerous. It appears the attempt is to put a null > terminator on the end, but not count it in the length of the vector. And I > guess that we know it's safe here because path is an llvm::SmallVector, so > the implementation details are known. But, ugh. If `path` were ever changed > to std::vector, we'd no longer have assurance that the terminator remains > after the pop. I totally agree. Took me a few minutes before I figured out what it was doing the first time I saw it :-) This is also done in the existing `convertUTF8ToUTF16String` wrapper too.
All the implementations of `std::vector` that I know of will work with this, though as you say it's not guaranteed by the standard. Looking more closely, I think in this case it was only done for the call to `stat`, which I've removed. I had added it here too in case the caller relied on this null trick, but I don't think it's necessary in either place anymore. I'll remove it. ================ Comment at: lldb/trunk/source/Host/windows/FileSystem.cpp:231 @@ -191,3 +230,3 @@ - char buf[PATH_MAX]; + wchar_t buf[PATH_MAX + 1]; // Subtract 1 from the path length since this function does not add a null terminator. ---------------- amccarth wrote: > I agree with Zach that a dynamic solution here is better. It's already icky > that we have a 32KB stack buffer here. Turning it into a 64KB +1B stack > buffer seem egregious. Hah, yes. I'll replace this with a vector to start with. http://reviews.llvm.org/D17107 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits