zturner added a comment. In http://reviews.llvm.org/D17107#349463, @cameron314 wrote:
> Thanks for all the feedback! I'll look into it in detail tomorrow. > > Perhaps it would make more sense if I clarified where I'm coming from with > this patch -- we use LLDB on Windows (with a custom backend) where I work, > and wanted to be able to run a program with a non-ASCII path (and similarly > place breakpoints in such files, etc.). So, I tried to make //minimally > invasive// changes across the codebase in order to accomplish this, using the > existing LLVM support functions as much as possible. This is why the changes > are not entirely consistent as a whole -- I preferred consistency with the > surrounding code (e.g. error handling and buffer allocation) over global > consistency. Just to be clear, I don't want you to not use llvm support functions. I just want to kind of hide them behind a common abstraction that removes some of the boilerplate just so that readability doesn't suffer too much. In that WinUtfString class I whipped up, I left two functions unimplemented. `ConvertToUtf16` and `ConvertToUtf8`. Those would still be implemented in terms of those llvm support functions. I'm also not saying that's the best possible solution since I kind of concoted it on the fly without too much attention to detail. But it makes the conversion points look much cleaner, and you can wrap up all your error handling logic in there (ignored in my 5-minute implementation), so that the error codes, messages, etc are always consistent when something fails. 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
