On Tue, Oct 6, 2009 at 3:10 PM, Jeff Trawick <[email protected]> wrote:
> On Tue, Oct 6, 2009 at 2:51 PM, Ruediger Pluem <[email protected]> wrote: > >> >> >> On 10/06/2009 10:52 AM, Joe Orton wrote: >> > On Sun, Oct 04, 2009 at 12:09:58PM -0000, [email protected] 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) { > but makes no sense to burn those syscalls in case this is what user was looking for ;) retract
