apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jeff Trawick <traw...@gmail.com>
Subject Re: svn commit: r821524 - in /apr/apr/trunk: CHANGES include/apr_network_io.h libapr.dsp network_io/beos/socketcommon.c network_io/os2/socket_util.c network_io/unix/socket_util.c test/sockchild.c test/testsock.c
Date Tue, 06 Oct 2009 19:12:42 GMT
On Tue, Oct 6, 2009 at 3:10 PM, Jeff Trawick <trawick@gmail.com> wrote:

> On Tue, Oct 6, 2009 at 2:51 PM, Ruediger Pluem <rpluem@apache.org> wrote:
>
>>
>>
>> On 10/06/2009 10:52 AM, Joe Orton wrote:
>> > On Sun, Oct 04, 2009 at 12:09:58PM -0000, rpluem@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) {
>

but makes no sense to burn those syscalls in case this is what user was
looking for ;)

retract

Mime
View raw message