Index: include/lldb/Core/Connection.h =================================================================== --- include/lldb/Core/Connection.h +++ include/lldb/Core/Connection.h @@ -101,7 +101,9 @@ IsConnected () const = 0; //------------------------------------------------------------------ - /// The read function that attempts to read from the connection. + /// The read function that attempts to read from the connection. The + /// function returns as soon as at least part of the data is + /// available. /// /// @param[in] dst /// A destination buffer that must be at least \a dst_len bytes @@ -136,6 +138,41 @@ Error *error_ptr) = 0; //------------------------------------------------------------------ + /// The read function that attempts to read from the connection. + /// This function will attempt to read the full buffer. If the + /// buffer cannot be filled, the function will set the connection + /// status to a value other than eConnectionStatusSuccess (the exact + /// value depends on the error cause). + /// + /// @param[in] dst + /// A destination buffer that must be at least \a dst_len bytes + /// long. + /// + /// @param[in] dst_len + /// The number of bytes to attempt to read, and also the max + /// number of bytes that can be placed into \a dst. + /// + /// @param[in] timeout_usec + /// The number of microseconds to wait for the data. + /// + /// @param[out] status + /// On return, indicates whether the call was successful or terminated + /// due to some error condition. + /// + /// @param[out] error_ptr + /// A pointer to an error object that should be given an + /// appropriate error value if this method returns zero. This + /// value can be NULL if the error value should be ignored. + /// + /// @return + /// The number of bytes actually read. + /// + /// @see size_t Communication::Read (void *, size_t, uint32_t); + //------------------------------------------------------------------ + size_t + ReadAll(void *dst, size_t dst_len, uint32_t timeout_usec, lldb::ConnectionStatus &status, Error *error_ptr); + + //------------------------------------------------------------------ /// The actual write function that attempts to write to the /// communications protocol. /// Index: source/Core/Connection.cpp =================================================================== --- source/Core/Connection.cpp +++ source/Core/Connection.cpp @@ -19,6 +19,7 @@ #include "lldb/Host/ConnectionFileDescriptor.h" +using namespace lldb; using namespace lldb_private; Connection::Connection () @@ -38,3 +39,31 @@ #endif return new ConnectionFileDescriptor(); } + +size_t +Connection::ReadAll(void *dst, size_t dst_len, uint32_t timeout_usec, ConnectionStatus &status, Error *error_ptr) +{ + char *read_buffer = static_cast(dst); + + size_t total_read_bytes = 0; + while (total_read_bytes < dst_len) + { + size_t read_bytes = + Read(read_buffer + total_read_bytes, dst_len - total_read_bytes, timeout_usec, status, error_ptr); + total_read_bytes += read_bytes; + switch (status) + { + case eConnectionStatusSuccess: + break; // Try again. + + case eConnectionStatusEndOfFile: + case eConnectionStatusError: + case eConnectionStatusTimedOut: + case eConnectionStatusNoConnection: + case eConnectionStatusLostConnection: + case eConnectionStatusInterrupted: + return total_read_bytes; + } + } + return total_read_bytes; +} Index: source/Plugins/Platform/Android/AdbClient.cpp =================================================================== --- source/Plugins/Platform/Android/AdbClient.cpp +++ source/Plugins/Platform/Android/AdbClient.cpp @@ -492,17 +492,12 @@ { Error error; ConnectionStatus status; - char *read_buffer = static_cast(buffer); - - size_t tota_read_bytes = 0; - while (tota_read_bytes < size) - { - auto read_bytes = m_conn.Read (read_buffer + tota_read_bytes, size - tota_read_bytes, kReadTimeout, status, &error); - if (error.Fail ()) - return error; - tota_read_bytes += read_bytes; - } - return error; + size_t read_bytes = m_conn.ReadAll(buffer, size, kReadTimeout, status, &error); + if (error.Fail()) + return error; + if (read_bytes < size) + return Error("Unable to read full buffer."); + return Error(); } Error Index: unittests/Core/CMakeLists.txt =================================================================== --- unittests/Core/CMakeLists.txt +++ unittests/Core/CMakeLists.txt @@ -1,4 +1,5 @@ add_lldb_unittest(LLDBCoreTests + ConnectionTest.cpp DataExtractorTest.cpp ScalarTest.cpp ) Index: unittests/Core/ConnectionTest.cpp =================================================================== --- /dev/null +++ unittests/Core/ConnectionTest.cpp @@ -0,0 +1,123 @@ +//===-- ConnectionTest.cpp --------------------------------------*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#if defined(_MSC_VER) && (_HAS_EXCEPTIONS == 0) +// Workaround for MSVC standard library bug, which fails to include when +// exceptions are disabled. +#include +#endif + +#include + +#include "gtest/gtest.h" + +#include "lldb/Core/Error.h" +#include "lldb/Host/ConnectionFileDescriptor.h" +#include "lldb/Host/common/TCPSocket.h" + +using namespace lldb_private; +using namespace lldb; + +class ConnectionTest : public testing::Test +{ +public: + void + SetUp() override + { +#if defined(_MSC_VER) + WSADATA data; + ::WSAStartup(MAKEWORD(2, 2), &data); +#endif + } + + void + TearDown() override + { +#if defined(_MSC_VER) + ::WSACleanup(); +#endif + } +}; + +TEST_F(ConnectionTest, ReadAll) +{ + const bool child_process_inherit = false; + Error error; + TCPSocket listen_socket(child_process_inherit, error); + ASSERT_TRUE(error.Success()) << error.AsCString(); + + error = listen_socket.Listen("localhost:0", 1); + ASSERT_TRUE(error.Success()) << error.AsCString(); + + uint16_t listen_port = listen_socket.GetLocalPortNumber(); + + std::unique_ptr accepted_socket; + ConnectionFileDescriptor conn; + { + // Scope for accepted_socket_ptr. + Socket *accepted_socket_ptr; + std::future future_error = std::async(std::launch::async, [&listen_socket, &accepted_socket_ptr]() { + return listen_socket.Accept("localhost:0", child_process_inherit, accepted_socket_ptr); + }); + + std::ostringstream connect_url; + connect_url << "connect://localhost:" << listen_port; + + conn.Connect(connect_url.str().c_str(), &error); + ASSERT_TRUE(error.Success()) << error.AsCString(); + ASSERT_TRUE(future_error.get().Success()) << future_error.get().AsCString(); + accepted_socket.reset(accepted_socket_ptr); + } + + // First, make sure ReadAll returns nothing. + const uint32_t k_reasonable_timeout_us = 10 * 1000; + char buffer[100]; + ConnectionStatus status; + size_t bytes_read = conn.ReadAll(buffer, sizeof buffer, k_reasonable_timeout_us, status, &error); + ASSERT_TRUE(error.Success()) << error.AsCString(); + ASSERT_EQ(eConnectionStatusTimedOut, status); + ASSERT_EQ(0u, bytes_read); + + // Write some data, and make sure it arrives. + const char data[] = {1, 2, 3, 4}; + size_t bytes_written = sizeof data; + error = accepted_socket->Write(data, bytes_written); + ASSERT_TRUE(error.Success()) << error.AsCString(); + ASSERT_EQ(sizeof data, bytes_written); + bytes_read = conn.ReadAll(buffer, sizeof data, k_reasonable_timeout_us, status, &error); + ASSERT_TRUE(error.Success()) << error.AsCString(); + ASSERT_EQ(eConnectionStatusSuccess, status); + ASSERT_EQ(sizeof data, bytes_read); + ASSERT_EQ(0, memcmp(buffer, data, sizeof data)); + memset(buffer, 0, sizeof buffer); + + // Write the data in two chunks. Make we read all of it. + std::future future_error = std::async(std::launch::async, [&accepted_socket, data]() { + size_t bytes_written = sizeof(data) / 2; + Error error = accepted_socket->Write(data, bytes_written); + if (error.Fail()) + return error; + std::this_thread::sleep_for(std::chrono::microseconds(k_reasonable_timeout_us / 10)); + bytes_written = sizeof(data) / 2; + return accepted_socket->Write(data + bytes_written, bytes_written); + }); + bytes_read = conn.ReadAll(buffer, sizeof data, k_reasonable_timeout_us, status, &error); + ASSERT_TRUE(error.Success()) << error.AsCString(); + ASSERT_EQ(eConnectionStatusSuccess, status); + ASSERT_EQ(sizeof data, bytes_read); + ASSERT_TRUE(future_error.get().Success()) << future_error.get().AsCString(); + ASSERT_EQ(0, memcmp(buffer, data, sizeof data)); + + // Close the remote end, make sure ReadAll result is reasonable. + accepted_socket.reset(); + bytes_read = conn.ReadAll(buffer, sizeof buffer, k_reasonable_timeout_us, status, &error); + ASSERT_TRUE(error.Success()) << error.AsCString(); + ASSERT_EQ(eConnectionStatusEndOfFile, status); + ASSERT_EQ(0u, bytes_read); +}