amccarth added a subscriber: amccarth.
amccarth added a comment.
I like that you're doing this, but ...
There's a ton of duplication here. I'd really like to see the conversions
centralized with consistent error handling. Cluttering up already-too-long
functions with more #ifdefs and a half dozen lines of boilerplate (that's
working at a much lower level of abstraction than the surrounding code) doesn't
seem like moving in the right direction.
================
Comment at: lldb/trunk/source/Core/ConnectionSharedMemory.cpp:140
@@ +139,3 @@
+ llvm::SmallVector<UTF16, 128> wname;
+ if (llvm::convertUTF8ToUTF16String(name, wname))
+ {
----------------
It's too bad there isn't an overload that gives the wide string as a
std::wstring. Then we wouldn't need all the ugly C-style casts of wname.data().
================
Comment at: lldb/trunk/source/Host/common/File.cpp:330
@@ +329,3 @@
+ }
+ m_descriptor = ::_wopen((const wchar_t *)wpath.data(), oflag, mode);
+#else
----------------
_wopen is officially deprecated. How about _wsopen_s?
================
Comment at: lldb/trunk/source/Host/common/File.cpp:357
@@ +356,3 @@
+ {
+ error.SetError(1, lldb::eErrorTypeGeneric);
+ return 0;
----------------
Why is the error reported here different than many of the other locations?
================
Comment at: lldb/trunk/source/Host/common/FileSpec.cpp:237
@@ +236,3 @@
+ int stat_result;
+#ifdef _WIN32
+ llvm::SmallVector<UTF16, 128> wpath;
----------------
Instead of another #ifdef here, can't we just re-use the GetFileStats function
above?
================
Comment at: lldb/trunk/source/Host/common/FileSpec.cpp:1141
@@ -1103,1 +1140,3 @@
+
WIN32_FIND_DATA ffd;
+ HANDLE hFind = FindFirstFileW((LPCWSTR)wszDir.data(), &ffd);
----------------
WIN32_FIND_DATATW to be consistent with calling the wide versions of the
functions explicitly.
================
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());
----------------
MAX_PATH in Windows. (And someday, we should get rid of these fixed buffers so
we can use UNC paths.)
================
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.
----------------
MAX_PATH
================
Comment at: lldb/trunk/source/Host/windows/Host.cpp:73
@@ -70,3 +72,3 @@
// Get the process image path. MAX_PATH isn't long enough, paths can
actually be up to 32KB.
- std::vector<char> buffer(32768);
+ std::vector<UTF16> buffer(PATH_MAX);
DWORD dwSize = buffer.size();
----------------
The comment above says why you can't use MAX_PATH/PATH_MAX. If that's no
longer relevant, please update the comment.
================
Comment at: lldb/trunk/source/Host/windows/Host.cpp:160
@@ -158,3 +159,3 @@
- std::vector<char> buffer(MAX_PATH);
+ std::vector<wchar_t> buffer(PATH_MAX);
DWORD chars_copied = 0;
----------------
MAX_PATH on Windows.
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
