From a3577e1ac558d83428c492e806549e278e5dc8fb Mon Sep 17 00:00:00 2001 From: Josh Gao Date: Mon, 5 Dec 2016 13:24:48 -0800 Subject: [PATCH] Revert "Revert "adb: move adb_strerror to sysdeps/win32/errno.cpp."" This reverts commit 43c02b27cd50a75f0fecb44e56a9bf32c6923aef. Test: mma Change-Id: I6b22ead8a4b964973ee2fdb8deba42bea74880cf --- adb/Android.mk | 2 + adb/file_sync_client.cpp | 2 - adb/sysdeps.h | 4 +- adb/sysdeps/errno.h | 25 +++++++++ adb/sysdeps/win32/errno.cpp | 95 ++++++++++++++++++++++++++++++++ adb/sysdeps/win32/errno_test.cpp | 51 +++++++++++++++++ adb/sysdeps_win32.cpp | 77 +------------------------- adb/sysdeps_win32_test.cpp | 30 ---------- 8 files changed, 175 insertions(+), 111 deletions(-) create mode 100644 adb/sysdeps/errno.h create mode 100644 adb/sysdeps/win32/errno.cpp create mode 100644 adb/sysdeps/win32/errno_test.cpp diff --git a/adb/Android.mk b/adb/Android.mk index be04cfa10..f9998e4ec 100644 --- a/adb/Android.mk +++ b/adb/Android.mk @@ -88,10 +88,12 @@ LIBADB_linux_SRC_FILES := \ LIBADB_windows_SRC_FILES := \ sysdeps_win32.cpp \ + sysdeps/win32/errno.cpp \ sysdeps/win32/stat.cpp \ usb_windows.cpp \ LIBADB_TEST_windows_SRCS := \ + sysdeps/win32/errno_test.cpp \ sysdeps_win32_test.cpp \ include $(CLEAR_VARS) diff --git a/adb/file_sync_client.cpp b/adb/file_sync_client.cpp index caa7a5f6b..cf03160ee 100644 --- a/adb/file_sync_client.cpp +++ b/adb/file_sync_client.cpp @@ -15,12 +15,10 @@ */ #include -#include #include #include #include #include -#include #include #include #include diff --git a/adb/sysdeps.h b/adb/sysdeps.h index 05d9fded1..654072c35 100644 --- a/adb/sysdeps.h +++ b/adb/sysdeps.h @@ -34,6 +34,7 @@ #include #include +#include "sysdeps/errno.h" #include "sysdeps/stat.h" /* @@ -361,9 +362,6 @@ inline void seekdir(DIR*, long) { #define getcwd adb_getcwd -char* adb_strerror(int err); -#define strerror adb_strerror - // Helper class to convert UTF-16 argv from wmain() to UTF-8 args that can be // passed to main(). class NarrowArgs { diff --git a/adb/sysdeps/errno.h b/adb/sysdeps/errno.h new file mode 100644 index 000000000..e7e9841ec --- /dev/null +++ b/adb/sysdeps/errno.h @@ -0,0 +1,25 @@ +/* + * Copyright (C) 2016 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#pragma once + +#include +#include + +#if defined(_WIN32) +char* adb_strerror(int err); +#define strerror adb_strerror +#endif diff --git a/adb/sysdeps/win32/errno.cpp b/adb/sysdeps/win32/errno.cpp new file mode 100644 index 000000000..a3b9d9b97 --- /dev/null +++ b/adb/sysdeps/win32/errno.cpp @@ -0,0 +1,95 @@ +/* + * Copyright (C) 2016 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "sysdeps/errno.h" + +#include + +#include + +// Overrides strerror() to handle error codes not supported by the Windows C +// Runtime (MSVCRT.DLL). +char* adb_strerror(int err) { +// sysdeps.h defines strerror to adb_strerror, but in this function, we +// want to call the real C Runtime strerror(). +#pragma push_macro("strerror") +#undef strerror + const int saved_err = errno; // Save because we overwrite it later. + + // Lookup the string for an unknown error. + char* errmsg = strerror(-1); + const std::string unknown_error = (errmsg == nullptr) ? "" : errmsg; + + // Lookup the string for this error to see if the C Runtime has it. + errmsg = strerror(err); + if (errmsg != nullptr && unknown_error != errmsg) { + // The CRT returned an error message and it is different than the error + // message for an unknown error, so it is probably valid, so use it. + } else { + // Check if we have a string for this error code. + const char* custom_msg = nullptr; + switch (err) { +#pragma push_macro("ERR") +#undef ERR +#define ERR(errnum, desc) case errnum: custom_msg = desc; break + // These error strings are from AOSP bionic/libc/include/sys/_errdefs.h. + // Note that these cannot be longer than 94 characters because we + // pass this to _strerror() which has that requirement. + ERR(ECONNRESET, "Connection reset by peer"); + ERR(EHOSTUNREACH, "No route to host"); + ERR(ENETDOWN, "Network is down"); + ERR(ENETRESET, "Network dropped connection because of reset"); + ERR(ENOBUFS, "No buffer space available"); + ERR(ENOPROTOOPT, "Protocol not available"); + ERR(ENOTCONN, "Transport endpoint is not connected"); + ERR(ENOTSOCK, "Socket operation on non-socket"); + ERR(EOPNOTSUPP, "Operation not supported on transport endpoint"); +#pragma pop_macro("ERR") + } + + if (custom_msg != nullptr) { + // Use _strerror() to write our string into the writable per-thread + // buffer used by strerror()/_strerror(). _strerror() appends the + // msg for the current value of errno, so set errno to a consistent + // value for every call so that our code-path is always the same. + errno = 0; + errmsg = _strerror(custom_msg); + const size_t custom_msg_len = strlen(custom_msg); + // Just in case _strerror() returned a read-only string, check if + // the returned string starts with our custom message because that + // implies that the string is not read-only. + if ((errmsg != nullptr) && !strncmp(custom_msg, errmsg, custom_msg_len)) { + // _strerror() puts other text after our custom message, so + // remove that by terminating after our message. + errmsg[custom_msg_len] = '\0'; + } else { + // For some reason nullptr was returned or a pointer to a + // read-only string was returned, so fallback to whatever + // strerror() can muster (probably "Unknown error" or some + // generic CRT error string). + errmsg = strerror(err); + } + } else { + // We don't have a custom message, so use whatever strerror(err) + // returned earlier. + } + } + + errno = saved_err; // restore + + return errmsg; +#pragma pop_macro("strerror") +} diff --git a/adb/sysdeps/win32/errno_test.cpp b/adb/sysdeps/win32/errno_test.cpp new file mode 100644 index 000000000..09ec52c86 --- /dev/null +++ b/adb/sysdeps/win32/errno_test.cpp @@ -0,0 +1,51 @@ +/* + * Copyright (C) 2016 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "sysdeps/errno.h" + +#include + +#include + +void TestAdbStrError(int err, const char* expected) { + errno = 12345; + const char* result = adb_strerror(err); + // Check that errno is not overwritten. + EXPECT_EQ(12345, errno); + EXPECT_STREQ(expected, result); +} + +TEST(sysdeps_win32, adb_strerror) { + // Test an error code that should not have a mapped string. Use an error + // code that is not used by the internal implementation of adb_strerror(). + TestAdbStrError(-2, "Unknown error"); + // adb_strerror() uses -1 internally, so test that it can still be passed + // as a parameter. + TestAdbStrError(-1, "Unknown error"); + // Test very big, positive unknown error. + TestAdbStrError(1000000, "Unknown error"); + + // Test success case. + // Wine returns "Success" for strerror(0), Windows returns "No error", so accept both. + std::string success = adb_strerror(0); + EXPECT_TRUE(success == "Success" || success == "No error") << "strerror(0) = " << success; + + // Test error that regular strerror() should have a string for. + TestAdbStrError(EPERM, "Operation not permitted"); + // Test error that regular strerror() doesn't have a string for, but that + // adb_strerror() returns. + TestAdbStrError(ECONNRESET, "Connection reset by peer"); +} diff --git a/adb/sysdeps_win32.cpp b/adb/sysdeps_win32.cpp index 4dd549dbd..a4b5e6978 100644 --- a/adb/sysdeps_win32.cpp +++ b/adb/sysdeps_win32.cpp @@ -477,81 +477,6 @@ int adb_close(int fd) return 0; } -// Overrides strerror() to handle error codes not supported by the Windows C -// Runtime (MSVCRT.DLL). -char* adb_strerror(int err) { - // sysdeps.h defines strerror to adb_strerror, but in this function, we - // want to call the real C Runtime strerror(). -#pragma push_macro("strerror") -#undef strerror - const int saved_err = errno; // Save because we overwrite it later. - - // Lookup the string for an unknown error. - char* errmsg = strerror(-1); - const std::string unknown_error = (errmsg == nullptr) ? "" : errmsg; - - // Lookup the string for this error to see if the C Runtime has it. - errmsg = strerror(err); - if (errmsg != nullptr && unknown_error != errmsg) { - // The CRT returned an error message and it is different than the error - // message for an unknown error, so it is probably valid, so use it. - } else { - // Check if we have a string for this error code. - const char* custom_msg = nullptr; - switch (err) { -#pragma push_macro("ERR") -#undef ERR -#define ERR(errnum, desc) case errnum: custom_msg = desc; break - // These error strings are from AOSP bionic/libc/include/sys/_errdefs.h. - // Note that these cannot be longer than 94 characters because we - // pass this to _strerror() which has that requirement. - ERR(ECONNRESET, "Connection reset by peer"); - ERR(EHOSTUNREACH, "No route to host"); - ERR(ENETDOWN, "Network is down"); - ERR(ENETRESET, "Network dropped connection because of reset"); - ERR(ENOBUFS, "No buffer space available"); - ERR(ENOPROTOOPT, "Protocol not available"); - ERR(ENOTCONN, "Transport endpoint is not connected"); - ERR(ENOTSOCK, "Socket operation on non-socket"); - ERR(EOPNOTSUPP, "Operation not supported on transport endpoint"); -#pragma pop_macro("ERR") - } - - if (custom_msg != nullptr) { - // Use _strerror() to write our string into the writable per-thread - // buffer used by strerror()/_strerror(). _strerror() appends the - // msg for the current value of errno, so set errno to a consistent - // value for every call so that our code-path is always the same. - errno = 0; - errmsg = _strerror(custom_msg); - const size_t custom_msg_len = strlen(custom_msg); - // Just in case _strerror() returned a read-only string, check if - // the returned string starts with our custom message because that - // implies that the string is not read-only. - if ((errmsg != nullptr) && - !strncmp(custom_msg, errmsg, custom_msg_len)) { - // _strerror() puts other text after our custom message, so - // remove that by terminating after our message. - errmsg[custom_msg_len] = '\0'; - } else { - // For some reason nullptr was returned or a pointer to a - // read-only string was returned, so fallback to whatever - // strerror() can muster (probably "Unknown error" or some - // generic CRT error string). - errmsg = strerror(err); - } - } else { - // We don't have a custom message, so use whatever strerror(err) - // returned earlier. - } - } - - errno = saved_err; // restore - - return errmsg; -#pragma pop_macro("strerror") -} - /**************************************************************************/ /**************************************************************************/ /***** *****/ @@ -565,7 +490,7 @@ char* adb_strerror(int err) { static void _socket_set_errno( const DWORD err ) { // Because the Windows C Runtime (MSVCRT.DLL) strerror() does not support a // lot of POSIX and socket error codes, some of the resulting error codes - // are mapped to strings by adb_strerror() above. + // are mapped to strings by adb_strerror(). switch ( err ) { case 0: errno = 0; break; // Don't map WSAEINTR since that is only for Winsock 1.1 which we don't use. diff --git a/adb/sysdeps_win32_test.cpp b/adb/sysdeps_win32_test.cpp index c3a3fd7c1..529b21215 100755 --- a/adb/sysdeps_win32_test.cpp +++ b/adb/sysdeps_win32_test.cpp @@ -70,36 +70,6 @@ TEST(sysdeps_win32, adb_getenv) { } } -void TestAdbStrError(int err, const char* expected) { - errno = 12345; - const char* result = adb_strerror(err); - // Check that errno is not overwritten. - EXPECT_EQ(12345, errno); - EXPECT_STREQ(expected, result); -} - -TEST(sysdeps_win32, adb_strerror) { - // Test an error code that should not have a mapped string. Use an error - // code that is not used by the internal implementation of adb_strerror(). - TestAdbStrError(-2, "Unknown error"); - // adb_strerror() uses -1 internally, so test that it can still be passed - // as a parameter. - TestAdbStrError(-1, "Unknown error"); - // Test very big, positive unknown error. - TestAdbStrError(1000000, "Unknown error"); - - // Test success case. - // Wine returns "Success" for strerror(0), Windows returns "No error", so accept both. - std::string success = adb_strerror(0); - EXPECT_TRUE(success == "Success" || success == "No error") << "strerror(0) = " << success; - - // Test error that regular strerror() should have a string for. - TestAdbStrError(EPERM, "Operation not permitted"); - // Test error that regular strerror() doesn't have a string for, but that - // adb_strerror() returns. - TestAdbStrError(ECONNRESET, "Connection reset by peer"); -} - TEST(sysdeps_win32, unix_isatty) { // stdin and stdout should be consoles. Use CONIN$ and CONOUT$ special files // so that we can test this even if stdin/stdout have been redirected. Read