[OPEN-ILS-DEV] socket_bundle.c

[Scott McKellar]

Here are some rambling questions and comments about socket_bundle.c.
Some of them reflect valid concerns and some of them merely reflect
my vast ignorance of sockets. but I don't know which is which.

-----------------

In socket_open_unix_server(), we call setsockopt() for the socket option
TCP_NODELAY.  However this option is specifically for TCP, and we are
applying it to a UNIX domain socket.  Does that make sense?

------------------

In socket_open_tcp_client(): why do we call bind(), asking for port zero,
before calling connect()?  It was my understanding that a client seeking
to open a connection doesn't normally care what port gets assigned to
the socket under the covers.

Similar considerations apply to socket_open_udp_client(). although in
this case (as is normal for UDP) there is no call to connect().

----------------

Why do we have the functions socket_open_udp_client() and
socket_open_udp_server()?  We don't call either of them from anywhere,
nor do we offer any other support for UDP.  Indeed nothing in the
entire source tree calls the functions sendto() or recvfrom(), which
are what one normally uses to send and receive UDP datagrams,
respectively.  Unless there are plans afoot to use them in the future,
I propose to eliminate these functions.

----------------

I also propose to eliminate socket_connected(), which we don't call
from anywhere, or at least to reform it.

At best, this function should be renamed, because it doesn't really
test whether a socket is connected.  The most it can do is determine
whether a file descriptor is valid.

It doesn't even do that reliably, because the call to select() can
fail for other reasons.  In particular it can fail if it is interrupted
by a signal, even when the file descriptor is valid.  It should
examine errno, and repeat if it sees EINTR.  There is also ENOMEM,
meaning that select() couldn't allocate memory for internal tables.
We probably can't do much about that, and probably wouldn't last much
longer anyway.

Because we pass NULL for the timeout value, we risk letting select()
block indefinitely.  We should pass a zero timeout.

-----------------

We pass a flags argument to _socket_send(), but in practice that argument
is always zero (although that appears not to have always been the case).
As a result, socket_send() is merely a tissue-thin wrapper for
_socket_send(), with an extra layer of function call that accomplishes
nothing but add overhead (unless the compiler can optimize it away).

I propose to combine socket_call() and _socket_call() into a single
function, with no flags argument.  Is there a reason not to do so?

----------------

Scott McKellar
http://home.swbell.net/mck9/ct/