[Lldb-commits] [PATCH] D61833: Fix IPv6 support on lldb-server platform

[António Afonso via Phabricator via lldb-commits]

aadsm created this revision.
aadsm added a reviewer: labath.
Herald added subscribers: lldb-commits, jfb, mgorny.
Herald added a project: LLDB.

This is a general fix for the ConnectionFileDescriptor class but my main motivation was to make lldb-server working with IPv6.
The connect URI can use square brackets ([]) to wrap the interface part of the URI (e.g.: <scheme>://[<interface>]:<port>). For IPv6 addresses this is a must since its ip can include colons and it will overlap with the port colon otherwise. The URIParser class parses the square brackets correctly but the ConnectionFileDescriptor doesn't generate them for IPv6 addresses making it impossible to connect to the gdb server when using this protocol.

How to reproduce the issue:

  $ lldb-server p --server --listen [::1]:8080
  ...
  $ lldb
  (lldb) platform select remote-macosx
  (lldb) platform connect connect://[::1]:8080
  (lldb) platform process -p <pid>
  error: unable to launch a GDB server on 'computer'

The server was actually launched we were just not able to connect to it. With this fix lldb will correctly connect.

I fixed this by wrapping the ip portion with []. It was a simple fix but the tests was what took more time to do.

@labath I need your help figuring out a good way to set these tests up. I wanted to reuse the CreateConnectedSockets part of the SocketTests. Initially I thought about creating a few functions in a test socket utility file, but then I realized the tests need to initialize the Socket plugin for these functions to work. In the end I created a new class (SocketBasedTest) so I can ensure this plugin is correctly initialized and teared down. However, I'm not a fan of subclassing as a means of code shared (as I prefer composition or delegation). What are your thoughts on this?

Another question I have (and this is more a n00b question as I don't have prior c++ experience), the CreateConnectedSockets functions that exist use unique pointers, and after reading the documentation about this I decided to use .release() so I could safely pass them to the ConnectionFileDescriptor in the test I created for it. Should I have changed the unique pointers to shared pointers instead?


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D61833

Files:
  lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp
  lldb/unittests/Host/CMakeLists.txt
  lldb/unittests/Host/ConnectionFileDescriptorTest.cpp
  lldb/unittests/Host/SocketBasedTest.cpp
  lldb/unittests/Host/SocketBasedTest.h
  lldb/unittests/Host/SocketTest.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D61833.199173.patch
Type: text/x-patch
Size: 13476 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20190512/0b03aed1/attachment.bin>