From 61b4e5513a433cd8b2d884006b5d195384601ca6 Mon Sep 17 00:00:00 2001 From: Andrew Schwartzmeyer Date: Wed, 1 Jul 2015 16:21:52 -0700 Subject: [PATCH 01/33] Update GetUserName signature --- src/impl/getusername.cpp | 2 +- src/impl/getusername.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/impl/getusername.cpp b/src/impl/getusername.cpp index b37b1cffc6..e911da41b1 100644 --- a/src/impl/getusername.cpp +++ b/src/impl/getusername.cpp @@ -1,7 +1,7 @@ #include "getusername.h" #include -BOOL GetUserName(WCHAR_T* userName, UINT32* maxLength) +BOOL GetUserName(WCHAR_T* lpBuffer, LPDWORD lpnSize) { return 0; } diff --git a/src/impl/getusername.h b/src/impl/getusername.h index 922a4e96ad..7650b94cc8 100644 --- a/src/impl/getusername.h +++ b/src/impl/getusername.h @@ -4,7 +4,7 @@ PAL_BEGIN_EXTERNC -BOOL GetUserName(WCHAR_T* userName, UINT32* maxLength); +BOOL GetUserName(WCHAR_T* lpBuffer, LPDWORD lpnSize); PAL_END_EXTERNC From 197938c6a64475cd4bb153de9cc151d47472342f Mon Sep 17 00:00:00 2001 From: Andrew Schwartzmeyer Date: Wed, 1 Jul 2015 16:24:36 -0700 Subject: [PATCH 02/33] Check locale in GetUserName --- src/impl/getusername.cpp | 27 ++++++++++++++++++++++++++- src/impl/pal.h | 1 + 2 files changed, 27 insertions(+), 1 deletion(-) diff --git a/src/impl/getusername.cpp b/src/impl/getusername.cpp index e911da41b1..839e85c470 100644 --- a/src/impl/getusername.cpp +++ b/src/impl/getusername.cpp @@ -1,8 +1,33 @@ #include "getusername.h" +#include +#include +#include #include +#include +using namespace std; + +const string utf8 = "UTF-8"; + +// https://msdn.microsoft.com/en-us/library/windows/desktop/ms724432(v=vs.85).aspx +// Sets errno to: +// ERROR_BAD_ENVIRONMENT - locale is not UTF-8 +// +// Returns: +// 1 - succeeded +// 0 - failed BOOL GetUserName(WCHAR_T* lpBuffer, LPDWORD lpnSize) { - return 0; + errno = 0; + + // Select locale from environment + setlocale(LC_ALL, ""); + // Check that locale is UTF-8 + if (nl_langinfo(CODESET) != utf8) { + errno = ERROR_BAD_ENVIRONMENT; + return 0; + } + + return 1; } diff --git a/src/impl/pal.h b/src/impl/pal.h index f981088843..06ff5602c6 100644 --- a/src/impl/pal.h +++ b/src/impl/pal.h @@ -71,6 +71,7 @@ #define FALSE 0 #define ERROR_INVALID_PARAMETER 87 #define ERROR_OUTOFMEMORY 14 + #define ERROR_BAD_ENVIRONMENT 0x0000000A #define MAX_PATH 0x00000104 typedef unsigned long long uint64; #endif From 83d2162dc16a525145d3e3c6ebc7ff9bde6eb9e5 Mon Sep 17 00:00:00 2001 From: Andrew Schwartzmeyer Date: Wed, 1 Jul 2015 16:24:58 -0700 Subject: [PATCH 03/33] Use getlogin_r() in GetUserName and map errno --- src/impl/getusername.cpp | 47 ++++++++++++++++++++++++++++++++++++++++ src/impl/pal.h | 5 +++++ 2 files changed, 52 insertions(+) diff --git a/src/impl/getusername.cpp b/src/impl/getusername.cpp index 839e85c470..55ca9c93ce 100644 --- a/src/impl/getusername.cpp +++ b/src/impl/getusername.cpp @@ -12,6 +12,13 @@ const string utf8 = "UTF-8"; // https://msdn.microsoft.com/en-us/library/windows/desktop/ms724432(v=vs.85).aspx // Sets errno to: // ERROR_BAD_ENVIRONMENT - locale is not UTF-8 +// ERROR_TOO_MANY_OPEN_FILES - already have the maximum allowed number of open files +// ERROR_NO_ASSOCIATION - calling process has no controlling terminal +// ERROR_INSUFFICIENT_BUFFER - buffer not large enough to hold username string +// ERROR_NO_SUCH_USER - there was no corresponding entry in the utmp-file +// ERROR_OUTOFMEMORY - insufficient memory to allocate passwd structure +// ERROR_NO_ASSOCIATION - standard input didn't refer to a terminal +// ERROR_INVALID_FUNCTION - getlogin_r() returned an unrecognized error code // // Returns: // 1 - succeeded @@ -28,6 +35,46 @@ BOOL GetUserName(WCHAR_T* lpBuffer, LPDWORD lpnSize) return 0; } + // Get username from system in a thread-safe manner + char userName[*lpnSize]; + int err = getlogin_r(userName, *lpnSize); + // Map errno to Win32 Error Codes + if (err != 0) { + switch (errno) { + case EMFILE: + case ENFILE: + errno = ERROR_TOO_MANY_OPEN_FILES; + break; + case ENXIO: + errno = ERROR_NO_ASSOCIATION; + break; + case ERANGE: + errno = ERROR_INSUFFICIENT_BUFFER; + break; + case ENOENT: + errno = ERROR_NO_SUCH_USER; + break; + case ENOMEM: + errno = ERROR_OUTOFMEMORY; + break; + case ENOTTY: + errno = ERROR_NO_ASSOCIATION; + break; + default: + errno = ERROR_INVALID_FUNCTION; + } + return 0; + } + + // TODO: Convert to char* to wchar* (UTF-8 to UTF-16 LE w/o BOM) + + // Unfortunately codecvt is not yet supported by G++, othwerise: + // wstring_convert,char16_t> conversion; + + // TODO: Use PAL's Unicode converters + + // TODO: set the lpnSize to the userName length (see msdn) + return 1; } diff --git a/src/impl/pal.h b/src/impl/pal.h index 06ff5602c6..f30903a2f4 100644 --- a/src/impl/pal.h +++ b/src/impl/pal.h @@ -72,6 +72,11 @@ #define ERROR_INVALID_PARAMETER 87 #define ERROR_OUTOFMEMORY 14 #define ERROR_BAD_ENVIRONMENT 0x0000000A + #define ERROR_TOO_MANY_OPEN_FILES 0x00000004 + #define ERROR_INSUFFICIENT_BUFFER 0x0000007A + #define ERROR_NO_ASSOCIATION 0x00000483 + #define ERROR_NO_SUCH_USER 0x00000525 + #define ERROR_INVALID_FUNCTION 0x00000001 #define MAX_PATH 0x00000104 typedef unsigned long long uint64; #endif From 95c6658178bda60883342084d973d16fd9eeec27 Mon Sep 17 00:00:00 2001 From: Andrew Schwartzmeyer Date: Mon, 20 Jul 2015 11:24:31 -0700 Subject: [PATCH 04/33] Add unit test for GetUserName --- src/CMakeLists.txt | 2 +- src/tests/test-getusername.cpp | 23 +++++++++++++++++++++++ 2 files changed, 24 insertions(+), 1 deletion(-) create mode 100644 src/tests/test-getusername.cpp diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index a5d27e7bfc..c350c6db02 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -11,7 +11,7 @@ link_directories(${monad_native_BINARY_DIR}) set(LIB_SOURCE_FILES impl/getcurrentprocessorid.cpp impl/getusername.cpp impl/terminal.cpp) set(HOST_COMMON_SOURCE_FILES host/common/coreclrutil.cpp) set(HOST_COMMON_TEST_SOURCE_FILES tests/host/test-hostutil.cpp) -set(TEST_SOURCE_FILES tests/test-getcurrentprocessid.cpp ${HOST_COMMON_SOURCE_FILES} ${HOST_COMMON_TEST_SOURCE_FILES}) +set(TEST_SOURCE_FILES tests/test-getcurrentprocessid.cpp tests/test-getusername.cpp ${HOST_COMMON_SOURCE_FILES} ${HOST_COMMON_TEST_SOURCE_FILES}) set(SOURCE_FILES main.cpp ../ext-src/gtest/fused-src/gtest/gtest-all.cc) SET(HOST_CMDLINE_SOURCE_FILES host/cmdline/main.cpp ${HOST_COMMON_SOURCE_FILES}) diff --git a/src/tests/test-getusername.cpp b/src/tests/test-getusername.cpp new file mode 100644 index 0000000000..1b15cc2d65 --- /dev/null +++ b/src/tests/test-getusername.cpp @@ -0,0 +1,23 @@ +#include +#include +#include "getusername.h" + +TEST(GetUserName,simple) +{ + // allocate a WCHAR_T buffer to receive username + DWORD lpnSize = 64; + WCHAR_T lpBuffer[lpnSize]; + + BOOL result = GetUserName(lpBuffer, &lpnSize); + + // GetUserName returns 1 on success + ASSERT_EQ(1, result); + + // get expected username + const char *username = getlogin(); + + // GetUserName sets lpnSize to length of username including null + ASSERT_EQ(strlen(username)+1, lpnSize); + + // TODO: ASSERT_STREQ(username, Utf16leToUtf8(lpBuffer)) +} From c8fa2152619286e2dc716c8c137a20061c4e0a2d Mon Sep 17 00:00:00 2001 From: Andrew Schwartzmeyer Date: Wed, 22 Jul 2015 10:06:31 -0700 Subject: [PATCH 05/33] Add ext-pal dependency Static library and header pulled from a build of the PAL on Ubuntu 14.04. Ugly workaround, but much easier than incorporating that entire project's build system, or even just a part of it. --- .gitmodules | 3 +++ ext-src/pal | 1 + 2 files changed, 4 insertions(+) create mode 160000 ext-src/pal diff --git a/.gitmodules b/.gitmodules index 63ba77f861..d01dc90650 100644 --- a/.gitmodules +++ b/.gitmodules @@ -1,3 +1,6 @@ [submodule "ext-src/gtest"] path = ext-src/gtest url = https://msostc.visualstudio.com/DefaultCollection/PS/_git/ext-gtest +[submodule "ext-src/pal"] + path = ext-src/pal + url = https://msostc.visualstudio.com/DefaultCollection/PS/_git/ext-pal diff --git a/ext-src/pal b/ext-src/pal new file mode 160000 index 0000000000..56765ad5b1 --- /dev/null +++ b/ext-src/pal @@ -0,0 +1 @@ +Subproject commit 56765ad5b11e3b061b5af45b74519ce5eda99127 From 0d9d31d5bdf4c3bcc63dba3baca41361a1078152 Mon Sep 17 00:00:00 2001 From: Andrew Schwartzmeyer Date: Wed, 22 Jul 2015 10:08:26 -0700 Subject: [PATCH 06/33] Link scxcore to ps and include headers --- src/CMakeLists.txt | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index c350c6db02..382bf39702 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -4,7 +4,7 @@ project(monad_native) set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++11") #include gtest -include_directories(../ext-src/gtest/fused-src impl) +include_directories(../ext-src/gtest/fused-src ../ext-src/pal/include impl) link_directories(${monad_native_BINARY_DIR}) # source file definitions @@ -17,6 +17,10 @@ SET(HOST_CMDLINE_SOURCE_FILES host/cmdline/main.cpp ${HOST_COMMON_SOURCE_FILES}) # target definitions add_library(ps SHARED ${LIB_SOURCE_FILES}) +add_library(scxcore STATIC IMPORTED GLOBAL) +set_property(TARGET scxcore PROPERTY IMPORTED_LOCATION ${CMAKE_SOURCE_DIR}/../ext-src/pal/lib/libscxcore.a) +target_link_libraries(ps scxcore) + add_executable(monad_native ${SOURCE_FILES} ${TEST_SOURCE_FILES}) add_executable(host_cmdline ${HOST_CMDLINE_SOURCE_FILES}) @@ -28,5 +32,3 @@ target_link_libraries(host_cmdline dl icuuc) # target specific include directories target_include_directories(monad_native PRIVATE host) target_include_directories(host_cmdline PRIVATE host) - - From 5f54d0c7484a504e8bed70e5930d10275c120606 Mon Sep 17 00:00:00 2001 From: Andrew Schwartzmeyer Date: Wed, 22 Jul 2015 10:15:34 -0700 Subject: [PATCH 07/33] Test UTF-16 output of GetUserName --- src/tests/test-getusername.cpp | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/src/tests/test-getusername.cpp b/src/tests/test-getusername.cpp index 1b15cc2d65..c2c9d1c2db 100644 --- a/src/tests/test-getusername.cpp +++ b/src/tests/test-getusername.cpp @@ -1,7 +1,14 @@ +#include +#include #include #include +#include #include "getusername.h" +using std::string; +using std::vector; +using SCXCoreLib::Utf16leToUtf8; + TEST(GetUserName,simple) { // allocate a WCHAR_T buffer to receive username @@ -14,10 +21,17 @@ TEST(GetUserName,simple) ASSERT_EQ(1, result); // get expected username - const char *username = getlogin(); + string username = string(getlogin()); // GetUserName sets lpnSize to length of username including null - ASSERT_EQ(strlen(username)+1, lpnSize); + ASSERT_EQ(username.size()+1, lpnSize); - // TODO: ASSERT_STREQ(username, Utf16leToUtf8(lpBuffer)) + // copy UTF-16 bytes from lpBuffer to vector for conversion + vector input(reinterpret_cast(&lpBuffer[0]), + reinterpret_cast(&lpBuffer[lpnSize-1])); + // convert to UTF-8 for assertion + string output; + Utf16leToUtf8(input, output); + + EXPECT_EQ(username, output); } From 69bfc8906ba5cfcbe8eb4de58bbc1b39a0c81bdb Mon Sep 17 00:00:00 2001 From: Andrew Schwartzmeyer Date: Wed, 22 Jul 2015 10:15:45 -0700 Subject: [PATCH 08/33] Explain GetUserName signature --- src/impl/getusername.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/impl/getusername.h b/src/impl/getusername.h index 7650b94cc8..50c6b1d5c9 100644 --- a/src/impl/getusername.h +++ b/src/impl/getusername.h @@ -4,7 +4,8 @@ PAL_BEGIN_EXTERNC -BOOL GetUserName(WCHAR_T* lpBuffer, LPDWORD lpnSize); +// WCHAR_T * is a Unicode LPTSTR +// See https://msdn.microsoft.com/en-us/library/windows/desktop/aa383751(v=vs.85).aspx#LPTSTR +BOOL GetUserName(WCHAR_T *lpBuffer, LPDWORD lpnSize); PAL_END_EXTERNC - From e14934d06d9c02ba060fdf367b06476ae29cd359 Mon Sep 17 00:00:00 2001 From: Andrew Schwartzmeyer Date: Wed, 22 Jul 2015 10:17:47 -0700 Subject: [PATCH 09/33] Check for invalid parameters in GetUserName --- src/impl/getusername.cpp | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/impl/getusername.cpp b/src/impl/getusername.cpp index 55ca9c93ce..5e3aeb1c5f 100644 --- a/src/impl/getusername.cpp +++ b/src/impl/getusername.cpp @@ -11,6 +11,7 @@ const string utf8 = "UTF-8"; // https://msdn.microsoft.com/en-us/library/windows/desktop/ms724432(v=vs.85).aspx // Sets errno to: +// ERROR_INVALID_PARAMETER - parameter is not valid // ERROR_BAD_ENVIRONMENT - locale is not UTF-8 // ERROR_TOO_MANY_OPEN_FILES - already have the maximum allowed number of open files // ERROR_NO_ASSOCIATION - calling process has no controlling terminal @@ -23,10 +24,16 @@ const string utf8 = "UTF-8"; // Returns: // 1 - succeeded // 0 - failed -BOOL GetUserName(WCHAR_T* lpBuffer, LPDWORD lpnSize) +BOOL GetUserName(WCHAR_T *lpBuffer, LPDWORD lpnSize) { errno = 0; + // Check parameters + if (!lpBuffer || !lpnSize) { + errno = ERROR_INVALID_PARAMETER; + return 0; + } + // Select locale from environment setlocale(LC_ALL, ""); // Check that locale is UTF-8 From 00390847e6dacfde17cfa38634ce7895592089c6 Mon Sep 17 00:00:00 2001 From: Andrew Schwartzmeyer Date: Wed, 22 Jul 2015 10:20:50 -0700 Subject: [PATCH 10/33] Convert get_login_r to UTF-16 and return --- src/impl/getusername.cpp | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/src/impl/getusername.cpp b/src/impl/getusername.cpp index 5e3aeb1c5f..ba58182502 100644 --- a/src/impl/getusername.cpp +++ b/src/impl/getusername.cpp @@ -4,8 +4,13 @@ #include #include #include +#include +#include -using namespace std; +using std::string; +using std::vector; + +using SCXCoreLib::Utf8ToUtf16le; const string utf8 = "UTF-8"; @@ -73,15 +78,22 @@ BOOL GetUserName(WCHAR_T *lpBuffer, LPDWORD lpnSize) return 0; } - // TODO: Convert to char* to wchar* (UTF-8 to UTF-16 LE w/o BOM) + // Convert to char * to WCHAR_T * (UTF-8 to UTF-16 LE w/o BOM) + string input = string(userName); + vector output; + Utf8ToUtf16le(input, output); - // Unfortunately codecvt is not yet supported by G++, othwerise: - // wstring_convert,char16_t> conversion; + if (output.size()/2 + 1 > *lpnSize) { + errno = ERROR_INSUFFICIENT_BUFFER; + return 0; + } - // TODO: Use PAL's Unicode converters + // Add two null bytes (because it's UTF-16) + output.push_back('\0'); + output.push_back('\0'); - // TODO: set the lpnSize to the userName length (see msdn) + memcpy(lpBuffer, &output[0], output.size()); + *lpnSize = output.size()/2; return 1; } - From 80f8fff6f294e7764f8d2489f2d7d7f5b7a8aee5 Mon Sep 17 00:00:00 2001 From: Andrew Schwartzmeyer Date: Wed, 22 Jul 2015 11:24:57 -0700 Subject: [PATCH 11/33] Clarify lpBuffer layout in GetUserName unit test --- src/tests/test-getusername.cpp | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/tests/test-getusername.cpp b/src/tests/test-getusername.cpp index c2c9d1c2db..825e949608 100644 --- a/src/tests/test-getusername.cpp +++ b/src/tests/test-getusername.cpp @@ -26,9 +26,11 @@ TEST(GetUserName,simple) // GetUserName sets lpnSize to length of username including null ASSERT_EQ(username.size()+1, lpnSize); - // copy UTF-16 bytes from lpBuffer to vector for conversion - vector input(reinterpret_cast(&lpBuffer[0]), - reinterpret_cast(&lpBuffer[lpnSize-1])); + // copy UTF-16 bytes (excluding null) from lpBuffer to vector for conversion + unsigned char *begin = reinterpret_cast(&lpBuffer[0]); + // -1 to skip null; *2 because UTF-16 encodes two bytes per character + unsigned char *end = begin + (lpnSize-1)*2; + vector input(begin, end); // convert to UTF-8 for assertion string output; Utf16leToUtf8(input, output); From 14cec68d087c42f7dcba87ca0b1b33b7e2c6416e Mon Sep 17 00:00:00 2001 From: Andrew Schwartzmeyer Date: Wed, 22 Jul 2015 13:43:09 -0700 Subject: [PATCH 12/33] Change global to local static const To avoid polluting the global namespace without using a namespace. --- src/impl/getusername.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/impl/getusername.cpp b/src/impl/getusername.cpp index ba58182502..6f11b516ae 100644 --- a/src/impl/getusername.cpp +++ b/src/impl/getusername.cpp @@ -12,8 +12,6 @@ using std::vector; using SCXCoreLib::Utf8ToUtf16le; -const string utf8 = "UTF-8"; - // https://msdn.microsoft.com/en-us/library/windows/desktop/ms724432(v=vs.85).aspx // Sets errno to: // ERROR_INVALID_PARAMETER - parameter is not valid @@ -31,6 +29,8 @@ const string utf8 = "UTF-8"; // 0 - failed BOOL GetUserName(WCHAR_T *lpBuffer, LPDWORD lpnSize) { + static const std::string utf8 = "UTF-8"; + errno = 0; // Check parameters From 097d2e3821fcd246b274a86957d0f85e4e871342 Mon Sep 17 00:00:00 2001 From: Andrew Schwartzmeyer Date: Wed, 22 Jul 2015 13:45:25 -0700 Subject: [PATCH 13/33] Factor out using directives --- src/impl/getusername.cpp | 11 +++-------- src/tests/test-getusername.cpp | 12 ++++-------- 2 files changed, 7 insertions(+), 16 deletions(-) diff --git a/src/impl/getusername.cpp b/src/impl/getusername.cpp index 6f11b516ae..35c034fb9d 100644 --- a/src/impl/getusername.cpp +++ b/src/impl/getusername.cpp @@ -7,11 +7,6 @@ #include #include -using std::string; -using std::vector; - -using SCXCoreLib::Utf8ToUtf16le; - // https://msdn.microsoft.com/en-us/library/windows/desktop/ms724432(v=vs.85).aspx // Sets errno to: // ERROR_INVALID_PARAMETER - parameter is not valid @@ -79,9 +74,9 @@ BOOL GetUserName(WCHAR_T *lpBuffer, LPDWORD lpnSize) } // Convert to char * to WCHAR_T * (UTF-8 to UTF-16 LE w/o BOM) - string input = string(userName); - vector output; - Utf8ToUtf16le(input, output); + std::string input(userName); + std::vector output; + SCXCoreLib::Utf8ToUtf16le(input, output); if (output.size()/2 + 1 > *lpnSize) { errno = ERROR_INSUFFICIENT_BUFFER; diff --git a/src/tests/test-getusername.cpp b/src/tests/test-getusername.cpp index 825e949608..c7bbc14b47 100644 --- a/src/tests/test-getusername.cpp +++ b/src/tests/test-getusername.cpp @@ -5,10 +5,6 @@ #include #include "getusername.h" -using std::string; -using std::vector; -using SCXCoreLib::Utf16leToUtf8; - TEST(GetUserName,simple) { // allocate a WCHAR_T buffer to receive username @@ -21,7 +17,7 @@ TEST(GetUserName,simple) ASSERT_EQ(1, result); // get expected username - string username = string(getlogin()); + std::string username(getlogin()); // GetUserName sets lpnSize to length of username including null ASSERT_EQ(username.size()+1, lpnSize); @@ -30,10 +26,10 @@ TEST(GetUserName,simple) unsigned char *begin = reinterpret_cast(&lpBuffer[0]); // -1 to skip null; *2 because UTF-16 encodes two bytes per character unsigned char *end = begin + (lpnSize-1)*2; - vector input(begin, end); + std::vector input(begin, end); // convert to UTF-8 for assertion - string output; - Utf16leToUtf8(input, output); + std::string output; + SCXCoreLib::Utf16leToUtf8(input, output); EXPECT_EQ(username, output); } From 59eb8a5799c2a676d8e9d74fa3c1f5f07d96b9b1 Mon Sep 17 00:00:00 2001 From: Andrew Schwartzmeyer Date: Wed, 22 Jul 2015 13:48:03 -0700 Subject: [PATCH 14/33] Use L_cuserid instead of 64 and given lpnSize --- src/impl/getusername.cpp | 4 ++-- src/tests/test-getusername.cpp | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/impl/getusername.cpp b/src/impl/getusername.cpp index 35c034fb9d..f9c73280a4 100644 --- a/src/impl/getusername.cpp +++ b/src/impl/getusername.cpp @@ -43,8 +43,8 @@ BOOL GetUserName(WCHAR_T *lpBuffer, LPDWORD lpnSize) } // Get username from system in a thread-safe manner - char userName[*lpnSize]; - int err = getlogin_r(userName, *lpnSize); + char userName[L_cuserid]; + int err = getlogin_r(userName, L_cuserid); // Map errno to Win32 Error Codes if (err != 0) { switch (errno) { diff --git a/src/tests/test-getusername.cpp b/src/tests/test-getusername.cpp index c7bbc14b47..7e1badee0d 100644 --- a/src/tests/test-getusername.cpp +++ b/src/tests/test-getusername.cpp @@ -8,7 +8,7 @@ TEST(GetUserName,simple) { // allocate a WCHAR_T buffer to receive username - DWORD lpnSize = 64; + DWORD lpnSize = L_cuserid; WCHAR_T lpBuffer[lpnSize]; BOOL result = GetUserName(lpBuffer, &lpnSize); From edcb8b04023177ec916eac600731b805be4c3479 Mon Sep 17 00:00:00 2001 From: Andrew Schwartzmeyer Date: Wed, 22 Jul 2015 14:21:48 -0700 Subject: [PATCH 15/33] Use test fixture in GetUserName unit tests --- src/tests/test-getusername.cpp | 31 ++++++++++++++++++++----------- 1 file changed, 20 insertions(+), 11 deletions(-) diff --git a/src/tests/test-getusername.cpp b/src/tests/test-getusername.cpp index 7e1badee0d..a565aa31eb 100644 --- a/src/tests/test-getusername.cpp +++ b/src/tests/test-getusername.cpp @@ -5,22 +5,31 @@ #include #include "getusername.h" -TEST(GetUserName,simple) -{ - // allocate a WCHAR_T buffer to receive username - DWORD lpnSize = L_cuserid; - WCHAR_T lpBuffer[lpnSize]; +class GetUserNameTest : public ::testing::Test { +protected: + DWORD lpnSize; + std::vector lpBuffer; + BOOL result; + std::string userName; - BOOL result = GetUserName(lpBuffer, &lpnSize); + GetUserNameTest(): userName(std::string(getlogin())) {} + + void GetUserNameWithSize(DWORD size) { + lpnSize = size; + // allocate a WCHAR_T buffer to receive username + lpBuffer.assign(lpnSize, '\0'); + result = GetUserName(&lpBuffer[0], &lpnSize); + } +}; + +TEST_F(GetUserNameTest, NormalUse) { + GetUserNameWithSize(L_cuserid); // GetUserName returns 1 on success ASSERT_EQ(1, result); - // get expected username - std::string username(getlogin()); - // GetUserName sets lpnSize to length of username including null - ASSERT_EQ(username.size()+1, lpnSize); + ASSERT_EQ(userName.size()+1, lpnSize); // copy UTF-16 bytes (excluding null) from lpBuffer to vector for conversion unsigned char *begin = reinterpret_cast(&lpBuffer[0]); @@ -31,5 +40,5 @@ TEST(GetUserName,simple) std::string output; SCXCoreLib::Utf16leToUtf8(input, output); - EXPECT_EQ(username, output); + EXPECT_EQ(userName, output); } From 876a88c7d8c8be6da43624ed70c955cdb7a049d4 Mon Sep 17 00:00:00 2001 From: Andrew Schwartzmeyer Date: Wed, 22 Jul 2015 15:10:44 -0700 Subject: [PATCH 16/33] Set lpnSize when insufficient --- src/impl/getusername.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/impl/getusername.cpp b/src/impl/getusername.cpp index f9c73280a4..203ed5e7e2 100644 --- a/src/impl/getusername.cpp +++ b/src/impl/getusername.cpp @@ -80,6 +80,7 @@ BOOL GetUserName(WCHAR_T *lpBuffer, LPDWORD lpnSize) if (output.size()/2 + 1 > *lpnSize) { errno = ERROR_INSUFFICIENT_BUFFER; + *lpnSize = output.size()/2 + 1; return 0; } From e2d4ce0ab1e5563b8482adf4085bfe30b010549b Mon Sep 17 00:00:00 2001 From: Andrew Schwartzmeyer Date: Wed, 22 Jul 2015 15:21:55 -0700 Subject: [PATCH 17/33] Add more unit test coverage for GetUserName --- src/tests/test-getusername.cpp | 115 +++++++++++++++++++++++++++------ 1 file changed, 96 insertions(+), 19 deletions(-) diff --git a/src/tests/test-getusername.cpp b/src/tests/test-getusername.cpp index a565aa31eb..b58387b267 100644 --- a/src/tests/test-getusername.cpp +++ b/src/tests/test-getusername.cpp @@ -14,31 +14,108 @@ protected: GetUserNameTest(): userName(std::string(getlogin())) {} - void GetUserNameWithSize(DWORD size) { + void TestWithSize(DWORD size) { lpnSize = size; // allocate a WCHAR_T buffer to receive username lpBuffer.assign(lpnSize, '\0'); result = GetUserName(&lpBuffer[0], &lpnSize); } + + void TestSuccess() { + SCOPED_TRACE(""); + // returns 1 on success + EXPECT_EQ(1, result); + + // sets lpnSize to length of username + null + ASSERT_EQ(userName.size()+1, lpnSize); + + // copy UTF-16 bytes (excluding null) from lpBuffer to vector for conversion + unsigned char *begin = reinterpret_cast(&lpBuffer[0]); + // -1 to skip null; *2 because UTF-16 encodes two bytes per character + unsigned char *end = begin + (lpnSize-1)*2; + std::vector input(begin, end); + // convert to UTF-8 for comparison + std::string output; + SCXCoreLib::Utf16leToUtf8(input, output); + + EXPECT_EQ(userName, output); + } + + void TestInvalidParameter() { + SCOPED_TRACE(""); + + // returns 0 on failure + EXPECT_EQ(0, result); + + // sets errno to ERROR_INVALID_PARAMETER when lpBuffer is null + // (which is the case for an empty vector) + EXPECT_EQ(errno, ERROR_INVALID_PARAMETER); + } + + void TestInsufficientBuffer() { + SCOPED_TRACE(""); + + // returns 0 on failure + EXPECT_EQ(0, result); + + // sets errno to ERROR_INSUFFICIENT_BUFFER + EXPECT_EQ(errno, ERROR_INSUFFICIENT_BUFFER); + + // sets lpnSize to length of username + null + EXPECT_EQ(userName.size()+1, lpnSize); + } }; -TEST_F(GetUserNameTest, NormalUse) { - GetUserNameWithSize(L_cuserid); +TEST_F(GetUserNameTest, BufferAsNullButNotBufferSize) { + lpnSize = 1; + result = GetUserName(NULL, &lpnSize); - // GetUserName returns 1 on success - ASSERT_EQ(1, result); - - // GetUserName sets lpnSize to length of username including null - ASSERT_EQ(userName.size()+1, lpnSize); - - // copy UTF-16 bytes (excluding null) from lpBuffer to vector for conversion - unsigned char *begin = reinterpret_cast(&lpBuffer[0]); - // -1 to skip null; *2 because UTF-16 encodes two bytes per character - unsigned char *end = begin + (lpnSize-1)*2; - std::vector input(begin, end); - // convert to UTF-8 for assertion - std::string output; - SCXCoreLib::Utf16leToUtf8(input, output); - - EXPECT_EQ(userName, output); + TestInvalidParameter(); + // does not reset lpnSize + EXPECT_EQ(1, lpnSize); +} + +TEST_F(GetUserNameTest, BufferSizeAsNullButNotBuffer) { + lpBuffer.push_back('\0'); + result = GetUserName(&lpBuffer[0], NULL); + + TestInvalidParameter(); +} + +TEST_F(GetUserNameTest, BufferSizeAsZero) { + TestWithSize(0); + TestInvalidParameter(); + // does not reset lpnSize + EXPECT_EQ(0, lpnSize); +} + +TEST_F(GetUserNameTest, BufferSizeAsOne) { + // theoretically this should never fail because any non-empty + // username length will be >1 with trailing null + TestWithSize(1); + TestInsufficientBuffer(); +} + +TEST_F(GetUserNameTest, BufferSizeAsUserName) { + // the buffer is too small because this does not include null + TestWithSize(userName.size()); + TestInsufficientBuffer(); +} + +TEST_F(GetUserNameTest, BufferSizeAsUserNameMinusOne) { + // the buffer is also too small + TestWithSize(userName.size()-1); + TestInsufficientBuffer(); +} + +TEST_F(GetUserNameTest, BufferSizeAsUserNamePlusOne) { + // the buffer is exactly big enough will null + TestWithSize(userName.size()+1); + TestSuccess(); +} + +TEST_F(GetUserNameTest, L_cuseridSize) { + // L_cuserid is big enough to hold any username + TestWithSize(L_cuserid); + TestSuccess(); } From bb8d0b793e9e7c5060f42ae093024662f48c14ca Mon Sep 17 00:00:00 2001 From: Andrew Schwartzmeyer Date: Wed, 22 Jul 2015 16:04:11 -0700 Subject: [PATCH 18/33] Use LOGIN_NAME_MAX instead of L_cuserid Since L_cuserid is only for cuserid, not getlogin_r Unit test for L_cuserid removed because the macro may not be available on all platforms. --- src/impl/getusername.cpp | 4 ++-- src/tests/test-getusername.cpp | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/impl/getusername.cpp b/src/impl/getusername.cpp index 203ed5e7e2..d369a4a3d5 100644 --- a/src/impl/getusername.cpp +++ b/src/impl/getusername.cpp @@ -43,8 +43,8 @@ BOOL GetUserName(WCHAR_T *lpBuffer, LPDWORD lpnSize) } // Get username from system in a thread-safe manner - char userName[L_cuserid]; - int err = getlogin_r(userName, L_cuserid); + char userName[LOGIN_NAME_MAX]; + int err = getlogin_r(userName, LOGIN_NAME_MAX); // Map errno to Win32 Error Codes if (err != 0) { switch (errno) { diff --git a/src/tests/test-getusername.cpp b/src/tests/test-getusername.cpp index b58387b267..8e025a2de6 100644 --- a/src/tests/test-getusername.cpp +++ b/src/tests/test-getusername.cpp @@ -114,8 +114,8 @@ TEST_F(GetUserNameTest, BufferSizeAsUserNamePlusOne) { TestSuccess(); } -TEST_F(GetUserNameTest, L_cuseridSize) { - // L_cuserid is big enough to hold any username - TestWithSize(L_cuserid); +TEST_F(GetUserNameTest, BufferSizeAsLoginNameMax) { + // LoginNameMax is big enough to hold any username + TestWithSize(LOGIN_NAME_MAX); TestSuccess(); } From b36cdcdc1241a83a6176802a79d1b059bfb1f1c6 Mon Sep 17 00:00:00 2001 From: Andrew Schwartzmeyer Date: Wed, 22 Jul 2015 16:29:17 -0700 Subject: [PATCH 19/33] Use std::string instead of C99 dynamic array --- src/impl/getusername.cpp | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/src/impl/getusername.cpp b/src/impl/getusername.cpp index d369a4a3d5..baff324c3f 100644 --- a/src/impl/getusername.cpp +++ b/src/impl/getusername.cpp @@ -43,8 +43,8 @@ BOOL GetUserName(WCHAR_T *lpBuffer, LPDWORD lpnSize) } // Get username from system in a thread-safe manner - char userName[LOGIN_NAME_MAX]; - int err = getlogin_r(userName, LOGIN_NAME_MAX); + std::string username(LOGIN_NAME_MAX, '\0'); + int err = getlogin_r(&username[0], username.size()); // Map errno to Win32 Error Codes if (err != 0) { switch (errno) { @@ -73,10 +73,17 @@ BOOL GetUserName(WCHAR_T *lpBuffer, LPDWORD lpnSize) return 0; } + // "Trim" the username to the first trailing null because + // otherwise the std::string with repeated null characters is + // valid, and the conversion will still include all the null + // characters. Creating a std::string from the C string of the + // original effectively trims it to the first null, without + // the need to manually trim whitespace (nor using Boost). + username = std::string(username.c_str()); + // Convert to char * to WCHAR_T * (UTF-8 to UTF-16 LE w/o BOM) - std::string input(userName); std::vector output; - SCXCoreLib::Utf8ToUtf16le(input, output); + SCXCoreLib::Utf8ToUtf16le(username, output); if (output.size()/2 + 1 > *lpnSize) { errno = ERROR_INSUFFICIENT_BUFFER; From 865fd0a56be0fde4c559e72b8c7d4fdb63fc9187 Mon Sep 17 00:00:00 2001 From: Andrew Schwartzmeyer Date: Wed, 22 Jul 2015 16:36:06 -0700 Subject: [PATCH 20/33] Remove unnecessary static qualifier from const string --- src/impl/getusername.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/impl/getusername.cpp b/src/impl/getusername.cpp index baff324c3f..282cd930da 100644 --- a/src/impl/getusername.cpp +++ b/src/impl/getusername.cpp @@ -24,7 +24,7 @@ // 0 - failed BOOL GetUserName(WCHAR_T *lpBuffer, LPDWORD lpnSize) { - static const std::string utf8 = "UTF-8"; + const std::string utf8 = "UTF-8"; errno = 0; From d589ac1cbfa4e0f2225ab37c9f9c5fd3b5ea1b0e Mon Sep 17 00:00:00 2001 From: Andrew Schwartzmeyer Date: Wed, 22 Jul 2015 16:36:19 -0700 Subject: [PATCH 21/33] Refactor setting of lpnSize when buffer size is insufficient --- src/impl/getusername.cpp | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/impl/getusername.cpp b/src/impl/getusername.cpp index 282cd930da..2e1e1dd125 100644 --- a/src/impl/getusername.cpp +++ b/src/impl/getusername.cpp @@ -85,9 +85,15 @@ BOOL GetUserName(WCHAR_T *lpBuffer, LPDWORD lpnSize) std::vector output; SCXCoreLib::Utf8ToUtf16le(username, output); - if (output.size()/2 + 1 > *lpnSize) { + // The length is the number of characters in the string, which + // is half the string size because UTF-16 encodes two bytes + // per character, plus one for the trailing null. + const DWORD length = output.size()/2 + 1; + if (length > *lpnSize) { errno = ERROR_INSUFFICIENT_BUFFER; - *lpnSize = output.size()/2 + 1; + // Set lpnSize if buffer is too small to inform user + // of necessary size + *lpnSize = length; return 0; } From 011d8f41ea4a7b51d7f260d7d48d5e5d63da04b9 Mon Sep 17 00:00:00 2001 From: Andrew Schwartzmeyer Date: Thu, 23 Jul 2015 09:28:34 -0700 Subject: [PATCH 22/33] Rename userName in test to expectedUsername To clarify that it is for comparison; not the result of GetUserName. --- src/tests/test-getusername.cpp | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/tests/test-getusername.cpp b/src/tests/test-getusername.cpp index 8e025a2de6..4f249e1187 100644 --- a/src/tests/test-getusername.cpp +++ b/src/tests/test-getusername.cpp @@ -10,9 +10,9 @@ protected: DWORD lpnSize; std::vector lpBuffer; BOOL result; - std::string userName; + std::string expectedUsername; - GetUserNameTest(): userName(std::string(getlogin())) {} + GetUserNameTest(): expectedUsername(std::string(getlogin())) {} void TestWithSize(DWORD size) { lpnSize = size; @@ -27,7 +27,7 @@ protected: EXPECT_EQ(1, result); // sets lpnSize to length of username + null - ASSERT_EQ(userName.size()+1, lpnSize); + ASSERT_EQ(expectedUsername.size()+1, lpnSize); // copy UTF-16 bytes (excluding null) from lpBuffer to vector for conversion unsigned char *begin = reinterpret_cast(&lpBuffer[0]); @@ -38,7 +38,7 @@ protected: std::string output; SCXCoreLib::Utf16leToUtf8(input, output); - EXPECT_EQ(userName, output); + EXPECT_EQ(expectedUsername, output); } void TestInvalidParameter() { @@ -62,7 +62,7 @@ protected: EXPECT_EQ(errno, ERROR_INSUFFICIENT_BUFFER); // sets lpnSize to length of username + null - EXPECT_EQ(userName.size()+1, lpnSize); + EXPECT_EQ(expectedUsername.size()+1, lpnSize); } }; @@ -98,19 +98,19 @@ TEST_F(GetUserNameTest, BufferSizeAsOne) { TEST_F(GetUserNameTest, BufferSizeAsUserName) { // the buffer is too small because this does not include null - TestWithSize(userName.size()); + TestWithSize(expectedUsername.size()); TestInsufficientBuffer(); } TEST_F(GetUserNameTest, BufferSizeAsUserNameMinusOne) { // the buffer is also too small - TestWithSize(userName.size()-1); + TestWithSize(expectedUsername.size()-1); TestInsufficientBuffer(); } TEST_F(GetUserNameTest, BufferSizeAsUserNamePlusOne) { // the buffer is exactly big enough will null - TestWithSize(userName.size()+1); + TestWithSize(expectedUsername.size()+1); TestSuccess(); } From 84d24589befa85bb12685678dba477444e3b6669 Mon Sep 17 00:00:00 2001 From: Andrew Schwartzmeyer Date: Tue, 4 Aug 2015 13:53:44 -0700 Subject: [PATCH 23/33] Link ps against icu instead of scxcore Remove scxcore library from CMake --- src/CMakeLists.txt | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 382bf39702..439546e2a1 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -4,7 +4,7 @@ project(monad_native) set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++11") #include gtest -include_directories(../ext-src/gtest/fused-src ../ext-src/pal/include impl) +include_directories(../ext-src/gtest/fused-src impl) link_directories(${monad_native_BINARY_DIR}) # source file definitions @@ -17,9 +17,7 @@ SET(HOST_CMDLINE_SOURCE_FILES host/cmdline/main.cpp ${HOST_COMMON_SOURCE_FILES}) # target definitions add_library(ps SHARED ${LIB_SOURCE_FILES}) -add_library(scxcore STATIC IMPORTED GLOBAL) -set_property(TARGET scxcore PROPERTY IMPORTED_LOCATION ${CMAKE_SOURCE_DIR}/../ext-src/pal/lib/libscxcore.a) -target_link_libraries(ps scxcore) +target_link_libraries(ps icuuc) add_executable(monad_native ${SOURCE_FILES} ${TEST_SOURCE_FILES}) add_executable(host_cmdline ${HOST_CMDLINE_SOURCE_FILES}) From 052ecc801d1360722d02eee4f6022b7c016baa2d Mon Sep 17 00:00:00 2001 From: Andrew Schwartzmeyer Date: Tue, 4 Aug 2015 13:58:44 -0700 Subject: [PATCH 24/33] Use icu for Unicode conversion and fix lpnSize A careful reading of the GetUserName documentation and Windows Data Types says that lpnSize is "the number of TCHARs copied to the buffer...including the terminating null character," and a TCHAR in this environment is a CHAR because we assume UNICODE is always defined for CoreCLR. A CHAR is a byte. So lpSize is number of bytes, not number of UTF-16 characters as we previously believed. --- src/impl/getusername.cpp | 112 +++++++++++++++++++++++++-------------- 1 file changed, 73 insertions(+), 39 deletions(-) diff --git a/src/impl/getusername.cpp b/src/impl/getusername.cpp index 2e1e1dd125..0314b0abed 100644 --- a/src/impl/getusername.cpp +++ b/src/impl/getusername.cpp @@ -4,24 +4,65 @@ #include #include #include -#include -#include +#include +#include +#include +#include -// https://msdn.microsoft.com/en-us/library/windows/desktop/ms724432(v=vs.85).aspx -// Sets errno to: -// ERROR_INVALID_PARAMETER - parameter is not valid -// ERROR_BAD_ENVIRONMENT - locale is not UTF-8 -// ERROR_TOO_MANY_OPEN_FILES - already have the maximum allowed number of open files -// ERROR_NO_ASSOCIATION - calling process has no controlling terminal -// ERROR_INSUFFICIENT_BUFFER - buffer not large enough to hold username string -// ERROR_NO_SUCH_USER - there was no corresponding entry in the utmp-file -// ERROR_OUTOFMEMORY - insufficient memory to allocate passwd structure -// ERROR_NO_ASSOCIATION - standard input didn't refer to a terminal -// ERROR_INVALID_FUNCTION - getlogin_r() returned an unrecognized error code -// -// Returns: -// 1 - succeeded -// 0 - failed +/* + GetUserName function + + https://msdn.microsoft.com/en-us/library/windows/desktop/ms724432(v=vs.85).aspx + + Retrieves the name of the user associated with the current thread. + + Parameters + + lpBuffer [out] + + A pointer to the buffer to receive the user's logon name. If this + buffer is not large enough to contain the entire user name, the + function fails. A buffer size of (UNLEN + 1) characters will hold + the maximum length user name including the terminating null + character. UNLEN is defined in Lmcons.h. + + lpnSize [in, out] + + On input, this variable specifies the size of the lpBuffer buffer, + in TCHARs. On output, the variable receives the number of TCHARs + copied to the buffer, including the terminating null character. + + Note that TCHAR is CHAR here because UNICODE is defined, and so a + TCHAR is a byte. + + https://msdn.microsoft.com/en-us/library/windows/desktop/aa383751(v=vs.85).aspx#CHAR + + If lpBuffer is too small, the function fails and GetLastError + returns ERROR_INSUFFICIENT_BUFFER. This parameter receives the + required buffer size, including the terminating null character. + + + Sets errno to: + ERROR_INVALID_PARAMETER - parameter is not valid + ERROR_BAD_ENVIRONMENT - locale is not UTF-8 + ERROR_TOO_MANY_OPEN_FILES - already have the maximum allowed number of open files + ERROR_NO_ASSOCIATION - calling process has no controlling terminal + ERROR_INSUFFICIENT_BUFFER - buffer not large enough to hold username string + ERROR_NO_SUCH_USER - there was no corresponding entry in the utmp-file + ERROR_OUTOFMEMORY - insufficient memory to allocate passwd structure + ERROR_NO_ASSOCIATION - standard input didn't refer to a terminal + ERROR_INVALID_FUNCTION - getlogin_r() returned an unrecognized error code + + Return value + + If the function succeeds, the return value is a nonzero value, and + the variable pointed to by lpnSize contains the number of TCHARs + copied to the buffer specified by lpBuffer, including the + terminating null character. + + If the function fails, the return value is zero. To get extended + error information, call GetLastError. +*/ BOOL GetUserName(WCHAR_T *lpBuffer, LPDWORD lpnSize) { const std::string utf8 = "UTF-8"; @@ -73,36 +114,29 @@ BOOL GetUserName(WCHAR_T *lpBuffer, LPDWORD lpnSize) return 0; } - // "Trim" the username to the first trailing null because - // otherwise the std::string with repeated null characters is - // valid, and the conversion will still include all the null - // characters. Creating a std::string from the C string of the - // original effectively trims it to the first null, without - // the need to manually trim whitespace (nor using Boost). - username = std::string(username.c_str()); - // Convert to char * to WCHAR_T * (UTF-8 to UTF-16 LE w/o BOM) - std::vector output; - SCXCoreLib::Utf8ToUtf16le(username, output); + std::basic_string username16(LOGIN_NAME_MAX+1, 0); + icu::UnicodeString username8(username.c_str(), "UTF-8"); + int32_t targetSize = username8.extract(0, username8.length(), + reinterpret_cast(&username16[0]), + (username16.size()-1)*sizeof(char16_t), + "UTF-16LE"); + // number of characters including null + username16.resize(targetSize/sizeof(char16_t)+1); - // The length is the number of characters in the string, which - // is half the string size because UTF-16 encodes two bytes - // per character, plus one for the trailing null. - const DWORD length = output.size()/2 + 1; - if (length > *lpnSize) { + // Size in bytes including null + const DWORD size = username16.length()*sizeof(char16_t); + if (size > *lpnSize) { errno = ERROR_INSUFFICIENT_BUFFER; // Set lpnSize if buffer is too small to inform user // of necessary size - *lpnSize = length; + *lpnSize = size; return 0; } - // Add two null bytes (because it's UTF-16) - output.push_back('\0'); - output.push_back('\0'); - - memcpy(lpBuffer, &output[0], output.size()); - *lpnSize = output.size()/2; + // Copy bytes from string to buffer + memcpy(lpBuffer, &username16[0], size); + *lpnSize = size; return 1; } From b77b0a7e6c56c4e0af239f0f572469c4f5b62247 Mon Sep 17 00:00:00 2001 From: Andrew Schwartzmeyer Date: Tue, 4 Aug 2015 14:02:34 -0700 Subject: [PATCH 25/33] Update GetUserName tests Note that these fail when run in a Docker container because a) the current container's locale is not completely set, and b) getlogin() returns null with ENOENT since /var/run/utmp is being mapped to the host, and thus denied access from within the container. The container needs the locale set and its own /run/utmp, see https://lists.linuxcontainers.org/pipermail/lxc-users/2011-July/002374.html --- src/tests/test-getusername.cpp | 69 ++++++++++++++++++++++------------ 1 file changed, 46 insertions(+), 23 deletions(-) diff --git a/src/tests/test-getusername.cpp b/src/tests/test-getusername.cpp index 4f249e1187..575aa85452 100644 --- a/src/tests/test-getusername.cpp +++ b/src/tests/test-getusername.cpp @@ -1,8 +1,10 @@ #include -#include #include #include -#include +#include +#include +#include +#include #include "getusername.h" class GetUserNameTest : public ::testing::Test { @@ -11,8 +13,11 @@ protected: std::vector lpBuffer; BOOL result; std::string expectedUsername; + DWORD expectedSize; - GetUserNameTest(): expectedUsername(std::string(getlogin())) {} + GetUserNameTest(): expectedUsername(std::string(getlogin())), + expectedSize((expectedUsername.length()+1)*sizeof(char16_t)) + {} void TestWithSize(DWORD size) { lpnSize = size; @@ -23,22 +28,28 @@ protected: void TestSuccess() { SCOPED_TRACE(""); + // returns 1 on success EXPECT_EQ(1, result); - // sets lpnSize to length of username + null - ASSERT_EQ(expectedUsername.size()+1, lpnSize); + // sets lpnSize to number of bytes including null, + // note that this is (length+1)*sizeof(char16_t) + ASSERT_EQ(expectedSize, lpnSize); - // copy UTF-16 bytes (excluding null) from lpBuffer to vector for conversion - unsigned char *begin = reinterpret_cast(&lpBuffer[0]); - // -1 to skip null; *2 because UTF-16 encodes two bytes per character - unsigned char *end = begin + (lpnSize-1)*2; - std::vector input(begin, end); - // convert to UTF-8 for comparison - std::string output; - SCXCoreLib::Utf16leToUtf8(input, output); + // setup for conversion from UTF-16LE + const char *begin = reinterpret_cast(&lpBuffer[0]); + icu::UnicodeString username16(begin, lpnSize, "UTF-16LE"); + // username16 length includes null and is number of characters + ASSERT_EQ(expectedUsername.length()+1, username16.length()); - EXPECT_EQ(expectedUsername, output); + // convert (minus null) to UTF-8 for comparison + std::string username(lpnSize/sizeof(char16_t)-1, 0); + ASSERT_EQ(expectedUsername.length(), username.length()); + int32_t targetSize = username16.extract(0, username.length(), + reinterpret_cast(&username[0]), + "UTF-8"); + + EXPECT_EQ(expectedUsername, username); } void TestInvalidParameter() { @@ -62,7 +73,7 @@ protected: EXPECT_EQ(errno, ERROR_INSUFFICIENT_BUFFER); // sets lpnSize to length of username + null - EXPECT_EQ(expectedUsername.size()+1, lpnSize); + EXPECT_EQ(expectedSize, lpnSize); } }; @@ -96,21 +107,33 @@ TEST_F(GetUserNameTest, BufferSizeAsOne) { TestInsufficientBuffer(); } -TEST_F(GetUserNameTest, BufferSizeAsUserName) { - // the buffer is too small because this does not include null +TEST_F(GetUserNameTest, BufferSizeAsUsername) { + // the buffer is too small because this is a UTF-8 size TestWithSize(expectedUsername.size()); TestInsufficientBuffer(); } -TEST_F(GetUserNameTest, BufferSizeAsUserNameMinusOne) { - // the buffer is also too small - TestWithSize(expectedUsername.size()-1); +TEST_F(GetUserNameTest, BufferSizeAsUsernamePlusOne) { + // the buffer is still too small even with null + TestWithSize(expectedUsername.size()+1); TestInsufficientBuffer(); } -TEST_F(GetUserNameTest, BufferSizeAsUserNamePlusOne) { - // the buffer is exactly big enough will null - TestWithSize(expectedUsername.size()+1); +TEST_F(GetUserNameTest, BufferSizeAsUsernameInUTF16) { + // the buffer is still too small because it is missing null + TestWithSize(expectedUsername.size()*sizeof(char16_t)); + TestInsufficientBuffer(); +} + +TEST_F(GetUserNameTest, BufferSizeAsUsernamePlusOneInUTF16) { + // the buffer is exactly big enough + TestWithSize((expectedUsername.size()+1)*sizeof(char16_t)); + TestSuccess(); +} + +TEST_F(GetUserNameTest, BufferSizeAsExpectedSize) { + // expectedSize is the same as username.size()+1 in UTF16 + TestWithSize(expectedSize); TestSuccess(); } From 7d007d67084ef9a4bce364af05d36a31619066b0 Mon Sep 17 00:00:00 2001 From: Andrew Schwartzmeyer Date: Tue, 4 Aug 2015 14:08:27 -0700 Subject: [PATCH 26/33] Remove PAL submodule --- .gitmodules | 3 --- ext-src/pal | 1 - 2 files changed, 4 deletions(-) delete mode 160000 ext-src/pal diff --git a/.gitmodules b/.gitmodules index d01dc90650..63ba77f861 100644 --- a/.gitmodules +++ b/.gitmodules @@ -1,6 +1,3 @@ [submodule "ext-src/gtest"] path = ext-src/gtest url = https://msostc.visualstudio.com/DefaultCollection/PS/_git/ext-gtest -[submodule "ext-src/pal"] - path = ext-src/pal - url = https://msostc.visualstudio.com/DefaultCollection/PS/_git/ext-pal diff --git a/ext-src/pal b/ext-src/pal deleted file mode 160000 index 56765ad5b1..0000000000 --- a/ext-src/pal +++ /dev/null @@ -1 +0,0 @@ -Subproject commit 56765ad5b11e3b061b5af45b74519ce5eda99127 From 2ec0c2996f8048d5feb84e660af674bd5528ec2b Mon Sep 17 00:00:00 2001 From: Andrew Schwartzmeyer Date: Tue, 4 Aug 2015 18:27:29 -0700 Subject: [PATCH 27/33] Use WCHAR sizes instead of CHAR sizes Such that lpnSize is the length (number of UTF-16 characters, including null) of the username. Must be multiplied by sizeof(char16_t) for bytes. --- src/impl/getusername.cpp | 10 +++++----- src/tests/test-getusername.cpp | 36 ++++++++++++---------------------- 2 files changed, 17 insertions(+), 29 deletions(-) diff --git a/src/impl/getusername.cpp b/src/impl/getusername.cpp index 0314b0abed..e2bd4e8377 100644 --- a/src/impl/getusername.cpp +++ b/src/impl/getusername.cpp @@ -32,7 +32,7 @@ in TCHARs. On output, the variable receives the number of TCHARs copied to the buffer, including the terminating null character. - Note that TCHAR is CHAR here because UNICODE is defined, and so a + Note that TCHAR is WCHAR here because UNICODE is defined, and so a TCHAR is a byte. https://msdn.microsoft.com/en-us/library/windows/desktop/aa383751(v=vs.85).aspx#CHAR @@ -121,11 +121,11 @@ BOOL GetUserName(WCHAR_T *lpBuffer, LPDWORD lpnSize) reinterpret_cast(&username16[0]), (username16.size()-1)*sizeof(char16_t), "UTF-16LE"); - // number of characters including null + // Number of characters including null username16.resize(targetSize/sizeof(char16_t)+1); - // Size in bytes including null - const DWORD size = username16.length()*sizeof(char16_t); + // Size in WCHARs including null + const DWORD size = username16.length(); if (size > *lpnSize) { errno = ERROR_INSUFFICIENT_BUFFER; // Set lpnSize if buffer is too small to inform user @@ -135,7 +135,7 @@ BOOL GetUserName(WCHAR_T *lpBuffer, LPDWORD lpnSize) } // Copy bytes from string to buffer - memcpy(lpBuffer, &username16[0], size); + memcpy(lpBuffer, &username16[0], size*sizeof(char16_t)); *lpnSize = size; return 1; diff --git a/src/tests/test-getusername.cpp b/src/tests/test-getusername.cpp index 575aa85452..1aab90721e 100644 --- a/src/tests/test-getusername.cpp +++ b/src/tests/test-getusername.cpp @@ -1,4 +1,5 @@ #include +#include #include #include #include @@ -16,7 +17,7 @@ protected: DWORD expectedSize; GetUserNameTest(): expectedUsername(std::string(getlogin())), - expectedSize((expectedUsername.length()+1)*sizeof(char16_t)) + expectedSize(expectedUsername.length()+1) {} void TestWithSize(DWORD size) { @@ -32,22 +33,21 @@ protected: // returns 1 on success EXPECT_EQ(1, result); - // sets lpnSize to number of bytes including null, - // note that this is (length+1)*sizeof(char16_t) + // sets lpnSize to number of WCHARs including null ASSERT_EQ(expectedSize, lpnSize); // setup for conversion from UTF-16LE const char *begin = reinterpret_cast(&lpBuffer[0]); - icu::UnicodeString username16(begin, lpnSize, "UTF-16LE"); + // multiply to get number of bytes + icu::UnicodeString username16(begin, lpnSize*sizeof(char16_t), "UTF-16LE"); // username16 length includes null and is number of characters - ASSERT_EQ(expectedUsername.length()+1, username16.length()); + ASSERT_EQ(expectedSize, username16.length()); // convert (minus null) to UTF-8 for comparison - std::string username(lpnSize/sizeof(char16_t)-1, 0); + std::string username(lpnSize-1, 0); ASSERT_EQ(expectedUsername.length(), username.length()); int32_t targetSize = username16.extract(0, username.length(), - reinterpret_cast(&username[0]), - "UTF-8"); + reinterpret_cast(&username[0]), "UTF-8"); EXPECT_EQ(expectedUsername, username); } @@ -109,30 +109,18 @@ TEST_F(GetUserNameTest, BufferSizeAsOne) { TEST_F(GetUserNameTest, BufferSizeAsUsername) { // the buffer is too small because this is a UTF-8 size - TestWithSize(expectedUsername.size()); + TestWithSize(expectedUsername.length()); TestInsufficientBuffer(); } TEST_F(GetUserNameTest, BufferSizeAsUsernamePlusOne) { - // the buffer is still too small even with null - TestWithSize(expectedUsername.size()+1); - TestInsufficientBuffer(); -} - -TEST_F(GetUserNameTest, BufferSizeAsUsernameInUTF16) { - // the buffer is still too small because it is missing null - TestWithSize(expectedUsername.size()*sizeof(char16_t)); - TestInsufficientBuffer(); -} - -TEST_F(GetUserNameTest, BufferSizeAsUsernamePlusOneInUTF16) { - // the buffer is exactly big enough - TestWithSize((expectedUsername.size()+1)*sizeof(char16_t)); + // includes null and so should be sufficient + TestWithSize(expectedUsername.length()+1); TestSuccess(); } TEST_F(GetUserNameTest, BufferSizeAsExpectedSize) { - // expectedSize is the same as username.size()+1 in UTF16 + // expectedSize is the same as username.size()+1 TestWithSize(expectedSize); TestSuccess(); } From afab62205adba68538fdef3dbdef5f57ec3c6c26 Mon Sep 17 00:00:00 2001 From: Andrew Schwartzmeyer Date: Thu, 6 Aug 2015 16:28:29 -0700 Subject: [PATCH 28/33] Rename to GetUserNameW since it is Unicode --- src/impl/getusername.cpp | 4 ++-- src/impl/getusername.h | 2 +- src/tests/test-getusername.cpp | 6 +++--- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/impl/getusername.cpp b/src/impl/getusername.cpp index e2bd4e8377..ba6f64ad86 100644 --- a/src/impl/getusername.cpp +++ b/src/impl/getusername.cpp @@ -10,7 +10,7 @@ #include /* - GetUserName function + GetUserNameW function https://msdn.microsoft.com/en-us/library/windows/desktop/ms724432(v=vs.85).aspx @@ -63,7 +63,7 @@ If the function fails, the return value is zero. To get extended error information, call GetLastError. */ -BOOL GetUserName(WCHAR_T *lpBuffer, LPDWORD lpnSize) +BOOL GetUserNameW(WCHAR_T *lpBuffer, LPDWORD lpnSize) { const std::string utf8 = "UTF-8"; diff --git a/src/impl/getusername.h b/src/impl/getusername.h index 50c6b1d5c9..ad6099c4b0 100644 --- a/src/impl/getusername.h +++ b/src/impl/getusername.h @@ -6,6 +6,6 @@ PAL_BEGIN_EXTERNC // WCHAR_T * is a Unicode LPTSTR // See https://msdn.microsoft.com/en-us/library/windows/desktop/aa383751(v=vs.85).aspx#LPTSTR -BOOL GetUserName(WCHAR_T *lpBuffer, LPDWORD lpnSize); +BOOL GetUserNameW(WCHAR_T *lpBuffer, LPDWORD lpnSize); PAL_END_EXTERNC diff --git a/src/tests/test-getusername.cpp b/src/tests/test-getusername.cpp index 1aab90721e..5a6a8535cc 100644 --- a/src/tests/test-getusername.cpp +++ b/src/tests/test-getusername.cpp @@ -24,7 +24,7 @@ protected: lpnSize = size; // allocate a WCHAR_T buffer to receive username lpBuffer.assign(lpnSize, '\0'); - result = GetUserName(&lpBuffer[0], &lpnSize); + result = GetUserNameW(&lpBuffer[0], &lpnSize); } void TestSuccess() { @@ -79,7 +79,7 @@ protected: TEST_F(GetUserNameTest, BufferAsNullButNotBufferSize) { lpnSize = 1; - result = GetUserName(NULL, &lpnSize); + result = GetUserNameW(NULL, &lpnSize); TestInvalidParameter(); // does not reset lpnSize @@ -88,7 +88,7 @@ TEST_F(GetUserNameTest, BufferAsNullButNotBufferSize) { TEST_F(GetUserNameTest, BufferSizeAsNullButNotBuffer) { lpBuffer.push_back('\0'); - result = GetUserName(&lpBuffer[0], NULL); + result = GetUserNameW(&lpBuffer[0], NULL); TestInvalidParameter(); } From 1b18bfd5e13e274a389a7b867a0476c8dbfdda02 Mon Sep 17 00:00:00 2001 From: Andrew Schwartzmeyer Date: Thu, 6 Aug 2015 17:03:53 -0700 Subject: [PATCH 29/33] Clean up CMakeLists --- src/CMakeLists.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 439546e2a1..1e6cafdbd6 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -13,17 +13,17 @@ set(HOST_COMMON_SOURCE_FILES host/common/coreclrutil.cpp) set(HOST_COMMON_TEST_SOURCE_FILES tests/host/test-hostutil.cpp) set(TEST_SOURCE_FILES tests/test-getcurrentprocessid.cpp tests/test-getusername.cpp ${HOST_COMMON_SOURCE_FILES} ${HOST_COMMON_TEST_SOURCE_FILES}) set(SOURCE_FILES main.cpp ../ext-src/gtest/fused-src/gtest/gtest-all.cc) -SET(HOST_CMDLINE_SOURCE_FILES host/cmdline/main.cpp ${HOST_COMMON_SOURCE_FILES}) +set(HOST_CMDLINE_SOURCE_FILES host/cmdline/main.cpp ${HOST_COMMON_SOURCE_FILES}) # target definitions add_library(ps SHARED ${LIB_SOURCE_FILES}) -target_link_libraries(ps icuuc) add_executable(monad_native ${SOURCE_FILES} ${TEST_SOURCE_FILES}) add_executable(host_cmdline ${HOST_CMDLINE_SOURCE_FILES}) # add pthread and other libs find_package(Threads) +target_link_libraries(ps icuuc) target_link_libraries(monad_native ${CMAKE_THREAD_LIBS_INIT} ps) target_link_libraries(host_cmdline dl icuuc) From 062eb5a7b4e8be20bbde962239abfc6843d10076 Mon Sep 17 00:00:00 2001 From: Andrew Schwartzmeyer Date: Thu, 6 Aug 2015 17:06:04 -0700 Subject: [PATCH 30/33] Fix style issues - Use Doxygen style comments (with better comments) - Use `type* ident` instead of `type *ident` - Start { on new lines - Use TRUE/FALSE from PAL.H --- src/impl/getusername.cpp | 127 ++++++++++++++++----------------- src/impl/getusername.h | 4 +- src/tests/test-getusername.cpp | 84 ++++++++++++++-------- 3 files changed, 119 insertions(+), 96 deletions(-) diff --git a/src/impl/getusername.cpp b/src/impl/getusername.cpp index ba6f64ad86..eb35137bde 100644 --- a/src/impl/getusername.cpp +++ b/src/impl/getusername.cpp @@ -1,4 +1,7 @@ -#include "getusername.h" +//! @file getusername.cpp +//! @author Andrew Schwartzmeyer +//! @brief Implements GetUserName Win32 API + #include #include #include @@ -8,66 +11,62 @@ #include #include #include +#include "getusername.h" -/* - GetUserNameW function - - https://msdn.microsoft.com/en-us/library/windows/desktop/ms724432(v=vs.85).aspx - - Retrieves the name of the user associated with the current thread. - - Parameters - - lpBuffer [out] - - A pointer to the buffer to receive the user's logon name. If this - buffer is not large enough to contain the entire user name, the - function fails. A buffer size of (UNLEN + 1) characters will hold - the maximum length user name including the terminating null - character. UNLEN is defined in Lmcons.h. - - lpnSize [in, out] - - On input, this variable specifies the size of the lpBuffer buffer, - in TCHARs. On output, the variable receives the number of TCHARs - copied to the buffer, including the terminating null character. - - Note that TCHAR is WCHAR here because UNICODE is defined, and so a - TCHAR is a byte. - - https://msdn.microsoft.com/en-us/library/windows/desktop/aa383751(v=vs.85).aspx#CHAR - - If lpBuffer is too small, the function fails and GetLastError - returns ERROR_INSUFFICIENT_BUFFER. This parameter receives the - required buffer size, including the terminating null character. - - - Sets errno to: - ERROR_INVALID_PARAMETER - parameter is not valid - ERROR_BAD_ENVIRONMENT - locale is not UTF-8 - ERROR_TOO_MANY_OPEN_FILES - already have the maximum allowed number of open files - ERROR_NO_ASSOCIATION - calling process has no controlling terminal - ERROR_INSUFFICIENT_BUFFER - buffer not large enough to hold username string - ERROR_NO_SUCH_USER - there was no corresponding entry in the utmp-file - ERROR_OUTOFMEMORY - insufficient memory to allocate passwd structure - ERROR_NO_ASSOCIATION - standard input didn't refer to a terminal - ERROR_INVALID_FUNCTION - getlogin_r() returned an unrecognized error code - - Return value - - If the function succeeds, the return value is a nonzero value, and - the variable pointed to by lpnSize contains the number of TCHARs - copied to the buffer specified by lpBuffer, including the - terminating null character. - - If the function fails, the return value is zero. To get extended - error information, call GetLastError. -*/ -BOOL GetUserNameW(WCHAR_T *lpBuffer, LPDWORD lpnSize) +//! @brief GetUserName retrieves the name of the user associated with +//! the current thread. +//! +//! GetUserNameW is the Unicode variation. See [MSDN documentation]. +//! +//! @param[out] lpBuffer +//! @parblock +//! A pointer to the buffer to receive the user's +//! logon name. If this buffer is not large enough to contain the +//! entire user name, the function fails. +//! +//! WCHAR_T* is a Unicode [LPTSTR]. +//! @endparblock +//! +//! @param[in, out] lpnSize +//! @parblock +//! On input, this variable specifies the size of the lpBuffer buffer, +//! in TCHARs. On output, the variable receives the number of TCHARs +//! copied to the buffer, including the terminating null character. +//! +//! TCHAR is a Unicode 16-bit [WCHAR]. +//! +//! If lpBuffer is too small, the function fails and GetLastError +//! returns ERROR_INSUFFICIENT_BUFFER. This parameter receives the +//! required buffer size, including the terminating null character. +//! @endparblock +//! +//! @exception errno Passes these errors via errno to GetLastError: +//! - ERROR_INVALID_PARAMETER: parameter is not valid +//! - ERROR_BAD_ENVIRONMENT: locale is not UTF-8 +//! - ERROR_TOO_MANY_OPEN_FILES: already have the maximum allowed number of open files +//! - ERROR_NO_ASSOCIATION: calling process has no controlling terminal +//! - ERROR_INSUFFICIENT_BUFFER: buffer not large enough to hold username string +//! - ERROR_NO_SUCH_USER: there was no corresponding entry in the utmp-file +//! - ERROR_OUTOFMEMORY: insufficient memory to allocate passwd structure +//! - ERROR_NO_ASSOCIATION: standard input didn't refer to a terminal +//! - ERROR_INVALID_FUNCTION: getlogin_r() returned an unrecognized error code +//! +//! @retval 1 If the function succeeds, the return value is a nonzero +//! value, and the variable pointed to by lpnSize contains the number +//! of TCHARs copied to the buffer specified by lpBuffer, including +//! the terminating null character. +//! +//! @retval 0 If the function fails, the return value is zero. To get +//! extended error information, call GetLastError. +//! +//! [MSDN documentation]: https://msdn.microsoft.com/en-us/library/windows/desktop/ms724432(v=vs.85).aspx +//! [WCHAR]: https://msdn.microsoft.com/en-us/library/windows/desktop/aa383751(v=vs.85).aspx#WCHAR +//! [LPTSTR]: https://msdn.microsoft.com/en-us/library/windows/desktop/aa383751(v=vs.85).aspx#LPTSTR +BOOL GetUserNameW(WCHAR_T* lpBuffer, LPDWORD lpnSize) { const std::string utf8 = "UTF-8"; - errno = 0; + errno = FALSE; // Check parameters if (!lpBuffer || !lpnSize) { @@ -80,14 +79,14 @@ BOOL GetUserNameW(WCHAR_T *lpBuffer, LPDWORD lpnSize) // Check that locale is UTF-8 if (nl_langinfo(CODESET) != utf8) { errno = ERROR_BAD_ENVIRONMENT; - return 0; + return FALSE; } // Get username from system in a thread-safe manner std::string username(LOGIN_NAME_MAX, '\0'); - int err = getlogin_r(&username[0], username.size()); + int ret = getlogin_r(&username[0], username.size()); // Map errno to Win32 Error Codes - if (err != 0) { + if (ret) { switch (errno) { case EMFILE: case ENFILE: @@ -111,14 +110,14 @@ BOOL GetUserNameW(WCHAR_T *lpBuffer, LPDWORD lpnSize) default: errno = ERROR_INVALID_FUNCTION; } - return 0; + return FALSE; } - // Convert to char * to WCHAR_T * (UTF-8 to UTF-16 LE w/o BOM) + // Convert to char* to WCHAR_T* (UTF-8 to UTF-16 LE w/o BOM) std::basic_string username16(LOGIN_NAME_MAX+1, 0); icu::UnicodeString username8(username.c_str(), "UTF-8"); int32_t targetSize = username8.extract(0, username8.length(), - reinterpret_cast(&username16[0]), + reinterpret_cast(&username16[0]), (username16.size()-1)*sizeof(char16_t), "UTF-16LE"); // Number of characters including null @@ -138,5 +137,5 @@ BOOL GetUserNameW(WCHAR_T *lpBuffer, LPDWORD lpnSize) memcpy(lpBuffer, &username16[0], size*sizeof(char16_t)); *lpnSize = size; - return 1; + return TRUE; } diff --git a/src/impl/getusername.h b/src/impl/getusername.h index ad6099c4b0..6083078bc0 100644 --- a/src/impl/getusername.h +++ b/src/impl/getusername.h @@ -4,8 +4,6 @@ PAL_BEGIN_EXTERNC -// WCHAR_T * is a Unicode LPTSTR -// See https://msdn.microsoft.com/en-us/library/windows/desktop/aa383751(v=vs.85).aspx#LPTSTR -BOOL GetUserNameW(WCHAR_T *lpBuffer, LPDWORD lpnSize); +BOOL GetUserNameW(WCHAR_T* lpBuffer, LPDWORD lpnSize); PAL_END_EXTERNC diff --git a/src/tests/test-getusername.cpp b/src/tests/test-getusername.cpp index 5a6a8535cc..d00acdf30b 100644 --- a/src/tests/test-getusername.cpp +++ b/src/tests/test-getusername.cpp @@ -1,3 +1,7 @@ +//! @file test-getusername.cpp +//! @author Andrew Schwartzmeyer +//! @brief Unit tests for GetUserName + #include #include #include @@ -8,7 +12,9 @@ #include #include "getusername.h" -class GetUserNameTest : public ::testing::Test { +//! Test fixture for GetUserNameW +class GetUserNameTest : public ::testing::Test +{ protected: DWORD lpnSize; std::vector lpBuffer; @@ -20,24 +26,31 @@ protected: expectedSize(expectedUsername.length()+1) {} - void TestWithSize(DWORD size) { + //! Invokes GetUserNameW with lpnSize and lpBuffer, saves result. + //! + //! @param size Assigns to lpnSize and allocates lpBuffer with + //! size number of null characters. + void TestWithSize(DWORD size) + { lpnSize = size; // allocate a WCHAR_T buffer to receive username lpBuffer.assign(lpnSize, '\0'); result = GetUserNameW(&lpBuffer[0], &lpnSize); } - void TestSuccess() { + //! Checks the effects of GetUserNameW for success. + void TestSuccess() + { SCOPED_TRACE(""); - // returns 1 on success - EXPECT_EQ(1, result); + //! Returns TRUE on success. + EXPECT_EQ(TRUE, result); - // sets lpnSize to number of WCHARs including null + //! Sets lpnSize to number of WCHARs including null. ASSERT_EQ(expectedSize, lpnSize); // setup for conversion from UTF-16LE - const char *begin = reinterpret_cast(&lpBuffer[0]); + const char* begin = reinterpret_cast(&lpBuffer[0]); // multiply to get number of bytes icu::UnicodeString username16(begin, lpnSize*sizeof(char16_t), "UTF-16LE"); // username16 length includes null and is number of characters @@ -46,38 +59,44 @@ protected: // convert (minus null) to UTF-8 for comparison std::string username(lpnSize-1, 0); ASSERT_EQ(expectedUsername.length(), username.length()); - int32_t targetSize = username16.extract(0, username.length(), - reinterpret_cast(&username[0]), "UTF-8"); + username16.extract(0, username.length(), + reinterpret_cast(&username[0]), "UTF-8"); + //! Returned username (after conversion) is what was expected. EXPECT_EQ(expectedUsername, username); } - void TestInvalidParameter() { + //! Checks the effects of GetUserNameW on failure with invalid parameters. + void TestInvalidParameter() + { SCOPED_TRACE(""); - // returns 0 on failure - EXPECT_EQ(0, result); + //! Returns FALSE on failure. + EXPECT_EQ(FALSE, result); - // sets errno to ERROR_INVALID_PARAMETER when lpBuffer is null - // (which is the case for an empty vector) + //! Sets errno to ERROR_INVALID_PARAMETER when lpBuffer is null + //! (which is the case for an empty vector). EXPECT_EQ(errno, ERROR_INVALID_PARAMETER); } - void TestInsufficientBuffer() { + //! Checks the effects of GetUserNameW on failure with a buffer that is too small. + void TestInsufficientBuffer() + { SCOPED_TRACE(""); - // returns 0 on failure - EXPECT_EQ(0, result); + //! Returns FALSE on failure. + EXPECT_EQ(FALSE, result); - // sets errno to ERROR_INSUFFICIENT_BUFFER + //! Sets errno to ERROR_INSUFFICIENT_BUFFER. EXPECT_EQ(errno, ERROR_INSUFFICIENT_BUFFER); - // sets lpnSize to length of username + null + //! Sets lpnSize to length of username plus null. EXPECT_EQ(expectedSize, lpnSize); } }; -TEST_F(GetUserNameTest, BufferAsNullButNotBufferSize) { +TEST_F(GetUserNameTest, BufferAsNullButNotBufferSize) +{ lpnSize = 1; result = GetUserNameW(NULL, &lpnSize); @@ -86,47 +105,54 @@ TEST_F(GetUserNameTest, BufferAsNullButNotBufferSize) { EXPECT_EQ(1, lpnSize); } -TEST_F(GetUserNameTest, BufferSizeAsNullButNotBuffer) { +TEST_F(GetUserNameTest, BufferSizeAsNullButNotBuffer) +{ lpBuffer.push_back('\0'); result = GetUserNameW(&lpBuffer[0], NULL); TestInvalidParameter(); } -TEST_F(GetUserNameTest, BufferSizeAsZero) { +TEST_F(GetUserNameTest, BufferSizeAsZero) +{ TestWithSize(0); TestInvalidParameter(); // does not reset lpnSize EXPECT_EQ(0, lpnSize); } -TEST_F(GetUserNameTest, BufferSizeAsOne) { +TEST_F(GetUserNameTest, BufferSizeAsOne) +{ // theoretically this should never fail because any non-empty // username length will be >1 with trailing null TestWithSize(1); TestInsufficientBuffer(); } -TEST_F(GetUserNameTest, BufferSizeAsUsername) { - // the buffer is too small because this is a UTF-8 size +TEST_F(GetUserNameTest, BufferSizeAsUsername) +{ + // the buffer is too small because it does not account for null TestWithSize(expectedUsername.length()); TestInsufficientBuffer(); } -TEST_F(GetUserNameTest, BufferSizeAsUsernamePlusOne) { +TEST_F(GetUserNameTest, BufferSizeAsUsernamePlusOne) +{ // includes null and so should be sufficient TestWithSize(expectedUsername.length()+1); TestSuccess(); } -TEST_F(GetUserNameTest, BufferSizeAsExpectedSize) { +TEST_F(GetUserNameTest, BufferSizeAsExpectedSize) +{ // expectedSize is the same as username.size()+1 TestWithSize(expectedSize); TestSuccess(); } -TEST_F(GetUserNameTest, BufferSizeAsLoginNameMax) { - // LoginNameMax is big enough to hold any username +TEST_F(GetUserNameTest, BufferSizeAsLoginNameMax) +{ + // LoginNameMax is big enough to hold any username, including null TestWithSize(LOGIN_NAME_MAX); TestSuccess(); } From 467fdeeb0ce1ceb5ca46f956ea0c673c5ec3d026 Mon Sep 17 00:00:00 2001 From: Andrew Schwartzmeyer Date: Fri, 7 Aug 2015 12:05:07 -0700 Subject: [PATCH 31/33] Map getlogin_r()'s ERANGE to ERROR_GEN_FAILURE ERROR_INSUFFICIENT_BUFFER should never occur here and would be a misleading error for the user. --- src/impl/getusername.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/impl/getusername.cpp b/src/impl/getusername.cpp index eb35137bde..0ca96e5a92 100644 --- a/src/impl/getusername.cpp +++ b/src/impl/getusername.cpp @@ -96,7 +96,7 @@ BOOL GetUserNameW(WCHAR_T* lpBuffer, LPDWORD lpnSize) errno = ERROR_NO_ASSOCIATION; break; case ERANGE: - errno = ERROR_INSUFFICIENT_BUFFER; + errno = ERROR_GEN_FAILURE; break; case ENOENT: errno = ERROR_NO_SUCH_USER; From f35efda7e782f7904d5ce4c0439058ac2b283d3c Mon Sep 17 00:00:00 2001 From: Andrew Schwartzmeyer Date: Fri, 7 Aug 2015 12:07:19 -0700 Subject: [PATCH 32/33] Add -Wall to CMAKE_CXX_FLAGS --- src/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 1e6cafdbd6..ae57dc376a 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -1,7 +1,7 @@ cmake_minimum_required(VERSION 2.8.4) project(monad_native) -set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++11") +set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++11 -Wall") #include gtest include_directories(../ext-src/gtest/fused-src impl) From a99c56eea3264b8c492bc0edf5e3a65102397af9 Mon Sep 17 00:00:00 2001 From: Andrew Schwartzmeyer Date: Fri, 7 Aug 2015 18:43:58 -0700 Subject: [PATCH 33/33] Convert to Allman style with 4-space indentation --- src/impl/getusername.cpp | 137 +++++++++++++------------ src/tests/test-getusername.cpp | 181 +++++++++++++++++---------------- 2 files changed, 162 insertions(+), 156 deletions(-) diff --git a/src/impl/getusername.cpp b/src/impl/getusername.cpp index 0ca96e5a92..5d0bec0361 100644 --- a/src/impl/getusername.cpp +++ b/src/impl/getusername.cpp @@ -64,78 +64,83 @@ //! [LPTSTR]: https://msdn.microsoft.com/en-us/library/windows/desktop/aa383751(v=vs.85).aspx#LPTSTR BOOL GetUserNameW(WCHAR_T* lpBuffer, LPDWORD lpnSize) { - const std::string utf8 = "UTF-8"; + const std::string utf8 = "UTF-8"; - errno = FALSE; + errno = FALSE; - // Check parameters - if (!lpBuffer || !lpnSize) { - errno = ERROR_INVALID_PARAMETER; - return 0; - } + // Check parameters + if (!lpBuffer || !lpnSize) + { + errno = ERROR_INVALID_PARAMETER; + return 0; + } - // Select locale from environment - setlocale(LC_ALL, ""); - // Check that locale is UTF-8 - if (nl_langinfo(CODESET) != utf8) { - errno = ERROR_BAD_ENVIRONMENT; - return FALSE; - } + // Select locale from environment + setlocale(LC_ALL, ""); + // Check that locale is UTF-8 + if (nl_langinfo(CODESET) != utf8) + { + errno = ERROR_BAD_ENVIRONMENT; + return FALSE; + } - // Get username from system in a thread-safe manner - std::string username(LOGIN_NAME_MAX, '\0'); - int ret = getlogin_r(&username[0], username.size()); - // Map errno to Win32 Error Codes - if (ret) { - switch (errno) { - case EMFILE: - case ENFILE: - errno = ERROR_TOO_MANY_OPEN_FILES; - break; - case ENXIO: - errno = ERROR_NO_ASSOCIATION; - break; - case ERANGE: - errno = ERROR_GEN_FAILURE; - break; - case ENOENT: - errno = ERROR_NO_SUCH_USER; - break; - case ENOMEM: - errno = ERROR_OUTOFMEMORY; - break; - case ENOTTY: - errno = ERROR_NO_ASSOCIATION; - break; - default: - errno = ERROR_INVALID_FUNCTION; - } - return FALSE; - } + // Get username from system in a thread-safe manner + std::string username(LOGIN_NAME_MAX, '\0'); + int ret = getlogin_r(&username[0], username.size()); + // Map errno to Win32 Error Codes + if (ret) + { + switch (errno) + { + case EMFILE: + case ENFILE: + errno = ERROR_TOO_MANY_OPEN_FILES; + break; + case ENXIO: + errno = ERROR_NO_ASSOCIATION; + break; + case ERANGE: + errno = ERROR_GEN_FAILURE; + break; + case ENOENT: + errno = ERROR_NO_SUCH_USER; + break; + case ENOMEM: + errno = ERROR_OUTOFMEMORY; + break; + case ENOTTY: + errno = ERROR_NO_ASSOCIATION; + break; + default: + errno = ERROR_INVALID_FUNCTION; + } + return FALSE; + } - // Convert to char* to WCHAR_T* (UTF-8 to UTF-16 LE w/o BOM) - std::basic_string username16(LOGIN_NAME_MAX+1, 0); - icu::UnicodeString username8(username.c_str(), "UTF-8"); - int32_t targetSize = username8.extract(0, username8.length(), - reinterpret_cast(&username16[0]), - (username16.size()-1)*sizeof(char16_t), - "UTF-16LE"); - // Number of characters including null - username16.resize(targetSize/sizeof(char16_t)+1); + // Convert to char* to WCHAR_T* (UTF-8 to UTF-16 LE w/o BOM) + std::basic_string username16(LOGIN_NAME_MAX+1, 0); + icu::UnicodeString username8(username.c_str(), "UTF-8"); + int32_t targetSize = username8.extract(0, username8.length(), + reinterpret_cast(&username16[0]), + (username16.size()-1)*sizeof(char16_t), + "UTF-16LE"); + // Number of characters including null + username16.resize(targetSize/sizeof(char16_t)+1); - // Size in WCHARs including null - const DWORD size = username16.length(); - if (size > *lpnSize) { - errno = ERROR_INSUFFICIENT_BUFFER; - // Set lpnSize if buffer is too small to inform user - // of necessary size - *lpnSize = size; - return 0; - } + // Size in WCHARs including null + const DWORD size = username16.length(); + if (size > *lpnSize) + { + errno = ERROR_INSUFFICIENT_BUFFER; + // Set lpnSize if buffer is too small to inform user + // of necessary size + *lpnSize = size; + return 0; + } - // Copy bytes from string to buffer - memcpy(lpBuffer, &username16[0], size*sizeof(char16_t)); - *lpnSize = size; + // Copy bytes from string to buffer + memcpy(lpBuffer, &username16[0], size*sizeof(char16_t)); + *lpnSize = size; - return TRUE; + return TRUE; } diff --git a/src/tests/test-getusername.cpp b/src/tests/test-getusername.cpp index d00acdf30b..e27aeef28b 100644 --- a/src/tests/test-getusername.cpp +++ b/src/tests/test-getusername.cpp @@ -16,143 +16,144 @@ class GetUserNameTest : public ::testing::Test { protected: - DWORD lpnSize; - std::vector lpBuffer; - BOOL result; - std::string expectedUsername; - DWORD expectedSize; + DWORD lpnSize; + std::vector lpBuffer; + BOOL result; + std::string expectedUsername; + DWORD expectedSize; - GetUserNameTest(): expectedUsername(std::string(getlogin())), - expectedSize(expectedUsername.length()+1) - {} + GetUserNameTest(): expectedUsername(std::string(getlogin())), + expectedSize(expectedUsername.length()+1) + { + } - //! Invokes GetUserNameW with lpnSize and lpBuffer, saves result. - //! - //! @param size Assigns to lpnSize and allocates lpBuffer with - //! size number of null characters. - void TestWithSize(DWORD size) - { - lpnSize = size; - // allocate a WCHAR_T buffer to receive username - lpBuffer.assign(lpnSize, '\0'); - result = GetUserNameW(&lpBuffer[0], &lpnSize); - } + //! Invokes GetUserNameW with lpnSize and lpBuffer, saves result. + //! + //! @param size Assigns to lpnSize and allocates lpBuffer with + //! size number of null characters. + void TestWithSize(DWORD size) + { + lpnSize = size; + // allocate a WCHAR_T buffer to receive username + lpBuffer.assign(lpnSize, '\0'); + result = GetUserNameW(&lpBuffer[0], &lpnSize); + } - //! Checks the effects of GetUserNameW for success. - void TestSuccess() - { - SCOPED_TRACE(""); + //! Checks the effects of GetUserNameW for success. + void TestSuccess() + { + SCOPED_TRACE(""); - //! Returns TRUE on success. - EXPECT_EQ(TRUE, result); + //! Returns TRUE on success. + EXPECT_EQ(TRUE, result); - //! Sets lpnSize to number of WCHARs including null. - ASSERT_EQ(expectedSize, lpnSize); + //! Sets lpnSize to number of WCHARs including null. + ASSERT_EQ(expectedSize, lpnSize); - // setup for conversion from UTF-16LE - const char* begin = reinterpret_cast(&lpBuffer[0]); - // multiply to get number of bytes - icu::UnicodeString username16(begin, lpnSize*sizeof(char16_t), "UTF-16LE"); - // username16 length includes null and is number of characters - ASSERT_EQ(expectedSize, username16.length()); + // setup for conversion from UTF-16LE + const char* begin = reinterpret_cast(&lpBuffer[0]); + // multiply to get number of bytes + icu::UnicodeString username16(begin, lpnSize*sizeof(char16_t), "UTF-16LE"); + // username16 length includes null and is number of characters + ASSERT_EQ(expectedSize, username16.length()); - // convert (minus null) to UTF-8 for comparison - std::string username(lpnSize-1, 0); - ASSERT_EQ(expectedUsername.length(), username.length()); - username16.extract(0, username.length(), - reinterpret_cast(&username[0]), "UTF-8"); + // convert (minus null) to UTF-8 for comparison + std::string username(lpnSize-1, 0); + ASSERT_EQ(expectedUsername.length(), username.length()); + username16.extract(0, username.length(), + reinterpret_cast(&username[0]), "UTF-8"); - //! Returned username (after conversion) is what was expected. - EXPECT_EQ(expectedUsername, username); - } + //! Returned username (after conversion) is what was expected. + EXPECT_EQ(expectedUsername, username); + } - //! Checks the effects of GetUserNameW on failure with invalid parameters. - void TestInvalidParameter() - { - SCOPED_TRACE(""); + //! Checks the effects of GetUserNameW on failure with invalid parameters. + void TestInvalidParameter() + { + SCOPED_TRACE(""); - //! Returns FALSE on failure. - EXPECT_EQ(FALSE, result); + //! Returns FALSE on failure. + EXPECT_EQ(FALSE, result); - //! Sets errno to ERROR_INVALID_PARAMETER when lpBuffer is null - //! (which is the case for an empty vector). - EXPECT_EQ(errno, ERROR_INVALID_PARAMETER); - } + //! Sets errno to ERROR_INVALID_PARAMETER when lpBuffer is null + //! (which is the case for an empty vector). + EXPECT_EQ(errno, ERROR_INVALID_PARAMETER); + } - //! Checks the effects of GetUserNameW on failure with a buffer that is too small. - void TestInsufficientBuffer() - { - SCOPED_TRACE(""); + //! Checks the effects of GetUserNameW on failure with a buffer that is too small. + void TestInsufficientBuffer() + { + SCOPED_TRACE(""); - //! Returns FALSE on failure. - EXPECT_EQ(FALSE, result); + //! Returns FALSE on failure. + EXPECT_EQ(FALSE, result); - //! Sets errno to ERROR_INSUFFICIENT_BUFFER. - EXPECT_EQ(errno, ERROR_INSUFFICIENT_BUFFER); + //! Sets errno to ERROR_INSUFFICIENT_BUFFER. + EXPECT_EQ(errno, ERROR_INSUFFICIENT_BUFFER); - //! Sets lpnSize to length of username plus null. - EXPECT_EQ(expectedSize, lpnSize); - } + //! Sets lpnSize to length of username plus null. + EXPECT_EQ(expectedSize, lpnSize); + } }; TEST_F(GetUserNameTest, BufferAsNullButNotBufferSize) { - lpnSize = 1; - result = GetUserNameW(NULL, &lpnSize); + lpnSize = 1; + result = GetUserNameW(NULL, &lpnSize); - TestInvalidParameter(); - // does not reset lpnSize - EXPECT_EQ(1, lpnSize); + TestInvalidParameter(); + // does not reset lpnSize + EXPECT_EQ(1, lpnSize); } TEST_F(GetUserNameTest, BufferSizeAsNullButNotBuffer) { - lpBuffer.push_back('\0'); - result = GetUserNameW(&lpBuffer[0], NULL); + lpBuffer.push_back('\0'); + result = GetUserNameW(&lpBuffer[0], NULL); - TestInvalidParameter(); + TestInvalidParameter(); } TEST_F(GetUserNameTest, BufferSizeAsZero) { - TestWithSize(0); - TestInvalidParameter(); - // does not reset lpnSize - EXPECT_EQ(0, lpnSize); + TestWithSize(0); + TestInvalidParameter(); + // does not reset lpnSize + EXPECT_EQ(0, lpnSize); } TEST_F(GetUserNameTest, BufferSizeAsOne) { - // theoretically this should never fail because any non-empty - // username length will be >1 with trailing null - TestWithSize(1); - TestInsufficientBuffer(); + // theoretically this should never fail because any non-empty + // username length will be >1 with trailing null + TestWithSize(1); + TestInsufficientBuffer(); } TEST_F(GetUserNameTest, BufferSizeAsUsername) { - // the buffer is too small because it does not account for null - TestWithSize(expectedUsername.length()); - TestInsufficientBuffer(); + // the buffer is too small because it does not account for null + TestWithSize(expectedUsername.length()); + TestInsufficientBuffer(); } TEST_F(GetUserNameTest, BufferSizeAsUsernamePlusOne) { - // includes null and so should be sufficient - TestWithSize(expectedUsername.length()+1); - TestSuccess(); + // includes null and so should be sufficient + TestWithSize(expectedUsername.length()+1); + TestSuccess(); } TEST_F(GetUserNameTest, BufferSizeAsExpectedSize) { - // expectedSize is the same as username.size()+1 - TestWithSize(expectedSize); - TestSuccess(); + // expectedSize is the same as username.size()+1 + TestWithSize(expectedSize); + TestSuccess(); } TEST_F(GetUserNameTest, BufferSizeAsLoginNameMax) { - // LoginNameMax is big enough to hold any username, including null - TestWithSize(LOGIN_NAME_MAX); - TestSuccess(); + // LoginNameMax is big enough to hold any username, including null + TestWithSize(LOGIN_NAME_MAX); + TestSuccess(); }