Mailing-List: contact dev-help@apr.apache.org; run by ezmlm
Precedence: bulk
Message-ID: <4ACF9A2D.8070706@apache.org>
Date: Fri, 09 Oct 2009 22:16:45 +0200
From: Ruediger Pluem <rpluem@apache.org>
User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US;
 rv:1.8.1.23) Gecko/20090823 SeaMonkey/1.1.18
MIME-Version: 1.0
To: dev@apr.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
References: <20091004120958.60C9B238890A@eris.apache.org>
 <20091006085245.GA5218@redhat.com> <4ACB91A7.3060700@apache.org>
 <4ACE3914.1070200@apache.org> <20091009082232.GA4340@redhat.com>
In-Reply-To: <20091009082232.GA4340@redhat.com>
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 8bit


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