A few comments:
- Any lldb_private forward declarations should be added to
"include/lldb/lldb-forward.h" and then that file should be included instead of
inlining any forward declarations manually. Also if you are going to use shared
pointers to then there is a section in lldb-forward.h at the bottom where you
should declare them like the others.
namespace lldb_private {
+class Error;
+class Socket;
- Can we rename IoObject to IOObject? Or is there a better name? IOConnection?
IOFile?
- Add the "_sp" suffix to each of these and use IOObjectSP that you define in
lldb-forward.h:
+ std::shared_ptr<IoObject> m_read;
+ std::shared_ptr<IoObject> m_write;
- We have been using the "_ap" suffix for auto pointers, but we are slowly
transitioning to using a "_up" for the unique pointers.
+ std::unique_ptr<Socket> m_listening_socket; // For listen/accept
connections, hold onto the
These make me nervous and should probably return a pointer or a shared pointer,
not a reference unless they are private and are known to be used only when
these are valid:
+ IoObject& GetReadObject() { return *m_read; }
+ const IoObject& GetReadObject() const { return *m_read; }
This kind of code seems like we are trying to make a new class IoObject, but
don't want to correctly abstract it down to a virtual class that can do
everything that we need of it. Seems like we should add a GetBoundPort(...) to
the IoObject class and have it return something invalid instead of casting and
having to know what the IoObject is and cast it:
+ if (read_object.GetFdType() == IoObject::eFDTypeSocket)
{
- char port_cstr[32];
- snprintf(port_cstr, sizeof(port_cstr), "127.0.0.1:%i",
out_port);
- // Send the host and port down that debugserver and specify an
option
- // so that it connects back to the port we are listening to in
this process
- debugserver_args.AppendArgument("--reverse-connect");
- debugserver_args.AppendArgument(port_cstr);
+ Socket& tcp_socket = static_cast<Socket&>(read_object);
+ assert(tcp_socket.GetSocketProtocol() == Socket::ProtocolTcp);
+ out_port = tcp_socket.GetBoundPort(10);
Other than that, lots of code has moved around into the host layer but it is
mostly the same. So looks good.
> On Jul 28, 2014, at 4:57 PM, Zachary Turner <[email protected]> wrote:
>
> This fixes the crashing in lldb-gdbserver, and should include all of the
> necessary things to get it to compile on MacOSX as well. (I have a request
> for my own Mac workstation pending, until then I have to cross my fingers and
> hope I did it right).
>
> lldb-gdbserver tests still fail, but now it's because of a pipe that's not
> getting any data. I suspect that all the remaining test failures now have
> the same underlying cause. Still investigating exactly what that might be,
> though.
>
> http://reviews.llvm.org/D4641
>
> Files:
> include/lldb/Core/ConnectionFileDescriptor.h
> include/lldb/Host/Editline.h
> include/lldb/Host/File.h
> include/lldb/Host/IoObject.h
> include/lldb/Host/Socket.h
> lldb.xcodeproj/project.pbxproj
> source/Core/ConnectionFileDescriptor.cpp
> source/Host/common/CMakeLists.txt
> source/Host/common/File.cpp
> source/Host/common/IoObject.cpp
> source/Host/common/Socket.cpp
> source/Plugins/Process/MacOSX-Kernel/ProcessKDP.cpp
> source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp
> tools/lldb-gdbserver/lldb-gdbserver.cpp
> tools/lldb-platform/lldb-platform.cpp
> <D4641.11964.patch>_______________________________________________
> lldb-commits mailing list
> [email protected]
> http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits
_______________________________________________
lldb-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/lldb-commits