mgorny created this revision. mgorny added reviewers: labath, krytarowski, emaste, teemperor. mgorny requested review of this revision.
Refactor ConnectToRemote() to improve readability and make future changes easier: 1. Replace static buffers with std::string. 2. When handling errors, prefer reporting the actual error over dumb 'connection status is not success'. 3. Move host/port parsing directly into reverse_connection condition that is its only user, and simplify it to make its purpose (verifying that a valid port is provided) clear. https://reviews.llvm.org/D111963 Files: lldb/tools/lldb-server/lldb-gdbserver.cpp
Index: lldb/tools/lldb-server/lldb-gdbserver.cpp =================================================================== --- lldb/tools/lldb-server/lldb-gdbserver.cpp +++ lldb/tools/lldb-server/lldb-gdbserver.cpp @@ -199,8 +199,7 @@ std::unique_ptr<Connection> connection_up; if (connection_fd != -1) { // Build the connection string. - char connection_url[512]; - snprintf(connection_url, sizeof(connection_url), "fd://%d", connection_fd); + std::string connection_url = llvm::formatv("fd://{0}", connection_fd).str(); // Create the connection. #if LLDB_ENABLE_POSIX && !defined _WIN32 @@ -208,23 +207,20 @@ #endif connection_up.reset(new ConnectionFileDescriptor); auto connection_result = connection_up->Connect(connection_url, &error); + if (error.Fail()) { + fprintf(stderr, "error: failed to connect to client at '%s': %s\n", + connection_url.c_str(), error.AsCString()); + exit(-1); + } if (connection_result != eConnectionStatusSuccess) { fprintf(stderr, "error: failed to connect to client at '%s' " "(connection status: %d)\n", - connection_url, static_cast<int>(connection_result)); - exit(-1); - } - if (error.Fail()) { - fprintf(stderr, "error: failed to connect to client at '%s': %s\n", - connection_url, error.AsCString()); + connection_url.c_str(), static_cast<int>(connection_result)); exit(-1); } } else if (!host_and_port.empty()) { // Parse out host and port. std::string final_host_and_port; - std::string connection_host; - std::string connection_port; - uint32_t connection_portno = 0; // If host_and_port starts with ':', default the host to be "localhost" and // expect the remainder to be the port. @@ -232,20 +228,15 @@ final_host_and_port.append("localhost"); final_host_and_port.append(host_and_port.str()); - // Note: use rfind, because the host/port may look like "[::1]:12345". - const std::string::size_type colon_pos = final_host_and_port.rfind(':'); - if (colon_pos != std::string::npos) { - connection_host = final_host_and_port.substr(0, colon_pos); - connection_port = final_host_and_port.substr(colon_pos + 1); - // FIXME: improve error handling - llvm::to_integer(connection_port, connection_portno); - } - - if (reverse_connect) { // llgs will connect to the gdb-remote client. // Ensure we have a port number for the connection. + // Note: use rfind, because the host/port may look like "[::1]:12345". + uint32_t connection_portno = 0; + const std::string::size_type colon_pos = final_host_and_port.rfind(':'); + if (colon_pos != std::string::npos) + llvm::to_integer(final_host_and_port.substr(colon_pos + 1), connection_portno); if (connection_portno == 0) { fprintf(stderr, "error: port number must be specified on when using " "reverse connect\n"); @@ -253,22 +244,21 @@ } // Build the connection string. - char connection_url[512]; - snprintf(connection_url, sizeof(connection_url), "connect://%s", - final_host_and_port.c_str()); + final_host_and_port.insert(0, "connect://"); // Create the connection. connection_up.reset(new ConnectionFileDescriptor); - auto connection_result = connection_up->Connect(connection_url, &error); + auto connection_result = connection_up->Connect(final_host_and_port, + &error); + if (error.Fail()) { + fprintf(stderr, "error: failed to connect to client at '%s': %s\n", + final_host_and_port.c_str(), error.AsCString()); + exit(-1); + } if (connection_result != eConnectionStatusSuccess) { fprintf(stderr, "error: failed to connect to client at '%s' " "(connection status: %d)\n", - connection_url, static_cast<int>(connection_result)); - exit(-1); - } - if (error.Fail()) { - fprintf(stderr, "error: failed to connect to client at '%s': %s\n", - connection_url, error.AsCString()); + final_host_and_port.c_str(), static_cast<int>(connection_result)); exit(-1); } } else {
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits