tmds / Tmds.Systemd

.NET Core library for interacting with systemd
Other
128 stars 17 forks source link

Set socket type on received socket #6

Closed logankp closed 7 years ago

logankp commented 7 years ago

The received socket had type Unknown which didn't allow me to use NetworkStream to write to it. I now set the socket type base on GetSockOpt.

tmds commented 7 years ago

The value returned from getsockopt is the Linux value. It needs to be converted to the correct .NET enum value.

If we're setting socket type, we might as well set addressFamily and protocolType too.

The AF constants are already defined in the class. These are the Linux protocoltype and sockettype values:

SOCK_STREAM = 1,        /* Sequenced, reliable, connection-based
SOCK_DGRAM = 2,         /* Connectionless, unreliable datagrams
SOCK_RAW = 3,           /* Raw protocol interface.  */
SOCK_RDM = 4,           /* Reliably-delivered messages.  */
SOCK_SEQPACKET = 5,     /* Sequenced, reliable, connection-based,

IPPROTO_ICMP = 1,       /* Internet Control Message Protocol.  */
IPPROTO_TCP = 6,        /* Transmission Control Protocol.  */
IPPROTO_UDP = 17,       /* User Datagram Protocol.  */
IPPROTO_ICMPV6 = 58,    /* ICMPv6.  */

Can you add ConvertProtocolType, ConvertAddressFamily, ConvertSocketType methods? They can throw unsupported when the native value cannot be matched, e.g.

throw new NotSupportedException($"Unknown address family: SO_DOMAIN={domain}.");

Can you the assert the expected values in the ListenSocketCanAccept test?

logankp commented 7 years ago

I can make these changes when I get some time.

The socket types defined in the header seem to match the .net Enum values already so is conversion really necessary?

tmds commented 7 years ago

The socket types defined in the header seem to match the .net Enum values already so is conversion really necessary?

It's not necessary to make it work. Still, I prefer to do it that way, because this is also how .NET Core implements this and it throws for 'unknown' values.

tmds commented 7 years ago

Thank you @logankp !