[Lldb-commits] [lldb] r135363 - in /lldb/trunk: include/lldb/Core/ include/lldb/Target/ source/ source/Core/ source/Plugins/Process/MacOSX-Kernel/ source/Plugins/Process/MacOSX-User/source/ source/Plugins/Process/gdb-remote/ source/Target/

[Matt Johnson]

On 07/19/2011 11:49 AM, Greg Clayton wrote:
> I added a new SocketAddress class in the host layer for you and modified the ConnectionFileDescriptor to use it:
>
> % svn commit
> Sending        include/lldb/Core/ConnectionFileDescriptor.h
> Adding         include/lldb/Host/SocketAddress.h
> Sending        lldb.xcodeproj/project.pbxproj
> Sending        source/Core/ConnectionFileDescriptor.cpp
> Adding         source/Host/common/SocketAddress.cpp
> Transmitting file data .....
> Committed revision 135488.
>
> We can now isolate all host specific changes into "SocketAddress.cpp". I left our simple implementation as:
>
> socklen_t
> SocketAddress::GetLength () const
> {
>      return m_socket_addr.sa.sa_len;
> }
>
> Feel free to add #ifdef stuff and "do the right thing" for linux.
>

Will do.

> In LLDB we would like to have the Host layer (all files in "lldb/include/lldb/Host/*" and "source/Host/*" abstract us from the host layer where we need it. This is the perfect case right here. We currently haven't run into anything that we haven't been able to take care of with this type of abtraction (we haven't required autoconf, nor do I really want it if we can avoid it).
>
> Let me know if this works for you,
>

That sounds great.  I'm not an autotools zealot by any means; as long as 
we can reasonably solve things at the preprocessor level (rather than 
needing to try compiling test programs like autoconf does), I prefer to 
do so as well.  Making this another class in the Host layer is a better 
fit for LLDB's architecture (and arguably, most C++ programs) than 
putting more stuff in LLVM's configure.ac.

Thanks for doing this.  I'll come up with a patch once I get a bit more 
bandwidth and figure out if most/all Linux distros/libc's work the same 
way as Ubuntu/glibc w.r.t. sockaddr and sockaddr_in.

> Greg Clayton
>

-Matt
> On Jul 18, 2011, at 5:32 PM, Matt Johnson wrote:
>
>> Hi all,
>>
>> struct sockaddr and struct sockaddr_in don't have sa_len and sin_len members, respectively in glibc/Ubuntu 11.04, breaking the build on r135363:
>>
>> ConnectionFileDescriptor.cpp: In member function ‘virtual size_t lldb_private::ConnectionFileDescriptor::Read(void*, size_t, uint32_t, lldb::ConnectionStatus&, lldb_private::Error*)’:
>> ConnectionFileDescriptor.cpp:289:57: error: ‘struct sockaddr’ has no member named ‘sa_len’
>> ConnectionFileDescriptor.cpp: In member function ‘virtual size_t lldb_private::ConnectionFileDescriptor::Write(const void*, size_t, lldb::ConnectionStatus&, lldb_private::Error*)’:
>> ConnectionFileDescriptor.cpp:410:59: error: ‘struct sockaddr’ has no member named ‘sa_len’
>> ConnectionFileDescriptor.cpp: In member function ‘lldb::ConnectionStatus lldb_private::ConnectionFileDescriptor::ConnectUDP(const char*, lldb_private::Error*)’:
>> ConnectionFileDescriptor.cpp:850:13: error: ‘struct sockaddr_in’ has no member named ‘sin_len’
>>
>> glibc provides equivalent functionality to sa_len with a macro SA_LEN, which looks at the sa_family member to determine the length (see http://lists.debian.org/debian-ipv6/2001/06/msg00015.html). Fixing the build under glibc would involve #ifdefing out the first use of sa_len and the only use of sin_len, and replacing the second use of sa_len with the SA_LEN() macro.
>>
>> I'm not submitting a patch yet because I wanted to ask how we want to test for sa_len and sin_len. Other projects have done it using autoconf macros (see http://archives.seul.org/or/cvs/Jan-2009/msg00014.html http://hackage.haskell.org/trac/ghc/ticket/2326 ), but I can't figure out on inspection exactly how lldb's Makefile is generated. I suspect it relies on llvm's configure script, but am not sure. If so, that would be the appropriate place to put the member checks, but this may require lldb to roll forward its frozen llvm version.
>>
>> Thoughts?
>>
>> Best,
>> Matt
>>