apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ruediger Pluem <rpl...@apache.org>
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 Fri, 09 Oct 2009 20:16:45 GMT


On 10/09/2009 10:22 AM, Joe Orton wrote:
> On Thu, Oct 08, 2009 at 09:10:12PM +0200, Ruediger Pluem wrote:
>> Sorry for being impatient here, but Joe any comment on the above?
>> I would like to fix this.
> 
> Sorry for the delay.
> 
> The closest parallel to this function in APR is apr_socket_atmark().  So 
> I suggest this function should mirror that one as closely in possible.
> 
> How about this instead?

Thanks. General looks fine to me. Just one comment in the code.
Do you commit?


>   * Create apr_sockaddr_t from hostname, address family, and port.
> Index: network_io/unix/socket_util.c
> ===================================================================
> --- network_io/unix/socket_util.c	(revision 823151)
> +++ network_io/unix/socket_util.c	(working copy)
> @@ -17,41 +17,58 @@
>  #include "apr_network_io.h"
>  #include "apr_poll.h"
>  
> -int apr_socket_is_connected(apr_socket_t *socket)
> +apr_status_t apr_socket_atreadeof(apr_socket_t *sock, int *atreadeof)
>  {
>      apr_pollfd_t pfds[1];
> -    apr_status_t status;
> +    apr_status_t rv;
>      apr_int32_t  nfds;
>  
> +    /* The purpose here is to return APR_SUCCESS only in cases in
> +     * which it can be unambiguously determined whether or not the
> +     * socket will return EOF on next read.  In case of an unexpected
> +     * error, return that. */
> +
>      pfds[0].reqevents = APR_POLLIN;
>      pfds[0].desc_type = APR_POLL_SOCKET;
> -    pfds[0].desc.s = socket;
> +    pfds[0].desc.s = sock;
>  
>      do {
> -        status = apr_poll(&pfds[0], 1, &nfds, 0);
> -    } while (APR_STATUS_IS_EINTR(status));
> +        rv = apr_poll(&pfds[0], 1, &nfds, 0);
> +    } while (APR_STATUS_IS_EINTR(rv));
>  
> -    if (status == APR_SUCCESS && nfds == 1 &&
> -        pfds[0].rtnevents == APR_POLLIN) {
> +    if (rv == APR_TIMEUP) {

Shouldn't we use APR_STATUS_IS_TIMEUP(rv) here?


Regards

RĂ¼diger

Mime
View raw message