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
