From e4fd76b7398ec4657ea014ac7f28a19dbb0077c7 Mon Sep 17 00:00:00 2001 From: Aaron Date: Mon, 17 Aug 2015 17:02:40 -0700 Subject: [PATCH] Added changed from feedback from Andy --- CMakeLists.txt | 2 +- impl/getcomputername.cpp | 24 +++++------------------ impl/pal.h | 2 ++ tests/test-getcomputername.cpp | 36 +++++++++++++++++++--------------- 4 files changed, 28 insertions(+), 36 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 5ee935bc3..d799e590b 100644 --- a/CMakeLists.txt +++ b/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 impl/getcomputername.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 tests/test-getusername.cpp tests/test-getcomputername ${HOST_COMMON_SOURCE_FILES} ${HOST_COMMON_TEST_SOURCE_FILES}) +set(TEST_SOURCE_FILES tests/test-getcurrentprocessid.cpp tests/test-getusername.cpp tests/test-getcomputername.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/impl/getcomputername.cpp b/impl/getcomputername.cpp index 03a8cda0e..364179aa1 100644 --- a/impl/getcomputername.cpp +++ b/impl/getcomputername.cpp @@ -87,26 +87,12 @@ BOOL GetComputerNameW(WCHAR_T* lpBuffer, LPDWORD lpnSize) { switch (errno) { - case EMFILE: - case ENFILE: - errno = ERROR_TOO_MANY_OPEN_FILES; + case EFAULT: + errno = ERROR_INVALID_ADDRESS; 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; - errno = ERROR_NO_ASSOCIATION; + case ENAMETOOLONG: + errno = ERROR_BUFFER_OVERFLOW; + break; default: errno = ERROR_INVALID_FUNCTION; } diff --git a/impl/pal.h b/impl/pal.h index f30903a2f..3ef233fd6 100644 --- a/impl/pal.h +++ b/impl/pal.h @@ -78,6 +78,8 @@ #define ERROR_NO_SUCH_USER 0x00000525 #define ERROR_INVALID_FUNCTION 0x00000001 #define MAX_PATH 0x00000104 + #define ERROR_INVALID_ADDRESS 0x1e7 + #define ERROR_BUFFER_OVERFLOW 0x6F typedef unsigned long long uint64; #endif diff --git a/tests/test-getcomputername.cpp b/tests/test-getcomputername.cpp index 612655c22..21e8c5382 100644 --- a/tests/test-getcomputername.cpp +++ b/tests/test-getcomputername.cpp @@ -3,6 +3,7 @@ //! @brief Unit tests for GetComputerNameW #include +#include #include #include #include @@ -21,17 +22,21 @@ protected: //Get expected result from using linux call GetComputerNameTest() - { - char hostname[HOST_NAME_MAX]; - BOOL host = gethostname(hostname, sizeof hostname); - expectedComputerName = hostname; - expectedSize = expectedComputerName.length() + 1; + { + expectedComputerName.resize(HOST_NAME_MAX); + BOOL ret = gethostname(&expectedComputerName[0], expectedComputerName.length()); + EXPECT_EQ(ret, 0); + expectedSize = std::strlen(expectedComputerName.c_str()) + 1; + expectedComputerName.resize(expectedSize - 1); } - + + //! Invokes GetComputerNameW 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 DWORD buffer to receive computername lpBuffer.assign(lpnSize, 0); result = GetComputerNameW(&lpBuffer[0], &lpnSize); } @@ -49,12 +54,11 @@ protected: // Read lpBuffer into UnicodeString (without null) const char* begin = reinterpret_cast(&lpBuffer[0]); icu::UnicodeString computername16(begin, (lpnSize-1)*sizeof(UChar), "UTF-16LE"); - ASSERT_EQ(expectedComputerName.length(), computername16.length()); // Convert to UTF-8 for comparison std::string computername; computername16.toUTF8String(computername); - ASSERT_EQ(expectedComputerName.length(), computername.length()); + ASSERT_EQ(expectedSize, computername.length() + 1); //! Returned computername(after conversion) is what was expected. EXPECT_EQ(expectedComputerName, computername); @@ -64,8 +68,8 @@ protected: { 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 EXPECT_EQ(errno, ERROR_INVALID_PARAMETER); @@ -75,14 +79,14 @@ protected: { SCOPED_TRACE(""); - // returns 0 on failure - EXPECT_EQ(0, result); + // returns FALSE on failure + EXPECT_EQ(FALSE, result); // sets errno to ERROR_INSUFFICIENT_BUFFER EXPECT_EQ(errno, ERROR_INSUFFICIENT_BUFFER); // sets lpnSize to length of username + null - ASSERT_EQ(expectedComputerName.size()+1, lpnSize); + ASSERT_EQ(expectedSize, lpnSize); } }; @@ -111,14 +115,14 @@ TEST_F(GetComputerNameTest, BufferSizeAsZero) TEST_F(GetComputerNameTest, BufferSizeAsUserNameMinusOne) { // the buffer is also too small - TestWithSize(expectedComputerName.size()-1); + TestWithSize(expectedSize-1); TestInsufficientBuffer(); } TEST_F(GetComputerNameTest, BufferSizeAsUserNamePlusOne) { // the buffer is exactly big enough - TestWithSize(expectedComputerName.size()+1); + TestWithSize(expectedSize+1); TestSuccess(); }