On Tue, Oct 6, 2009 at 2:51 PM, Ruediger Pluem <rpl...@apache.org> wrote:
> > > On 10/06/2009 10:52 AM, Joe Orton wrote: > > On Sun, Oct 04, 2009 at 12:09:58PM -0000, rpl...@apache.org wrote: > >> Author: rpluem > >> Date: Sun Oct 4 12:09:57 2009 > >> New Revision: 821524 > >> > >> URL: http://svn.apache.org/viewvc?rev=821524&view=rev > >> Log: > >> * Add apr_socket_is_connected to detect whether the remote side of a > socket > >> is still open. The origin of apr_socket_is_connected is r473278 from > >> > http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/proxy/proxy_util.c > >> in httpd. > > > > The naming is not good here. A TCP socket is "connected" after the > > connection is successfully established via connect(), up until it is > > destroyed. From the name, I would expect this function to do would be > > to say whether a non-blocking connect() has completed. > > > > This function will reliably tell you that the receive part of the socket > > has been shut down by the peer, in the case that the socket's read > > buffer is empty. Notably the socket may still be "connected" in that > > state, albeit in half-duplex mode. > > > > I'd suggest a name like: > > > > apr_socket_atreadeof > > apr_socket_at_read_eof > > I am fine with changing the name. I let this thread sit some time > to give others the opportunity to chime in as I want to avoid changing > the name more than once. If no proposal comes up I am likely to go > in the apr_socket_at_read_eof direction. > > > or something similar? The API docs should reflect that the return value > > is 1-on-success/zero-on-failure (unusual for APR), and that the function > > does not block. > > I can invert the return value as well, but in this case shouldn't this > be reflected in the name as well? > So with inverted return value shouldn't it be > > apr_socket_at_read_not_eof > > or do you think I shouldn't return an int at all and return apr_status_t > instead > with APR_SUCCESS if the read side of our end of the socket is eof > (and leaving the name as apr_socket_at_read_eof in this case)? > silly thought from the crowd, in case there are other conditions we'd potentially want to report: if (apr_socket_status_get(sock, &status) != APR_SUCCESS || status & APR_SOCK_AT_EOF) { /* stop using */ } Doesn't "EOF" imply "read"?