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?
Index: include/apr_network_io.h
===================================================================
--- include/apr_network_io.h (revision 823151)
+++ include/apr_network_io.h (working copy)
@@ -375,17 +375,19 @@
apr_sockaddr_t *sa);
/**
- * Check whether the send part of the socket is still open on the
- * peer or that there is still data in the socket's read buffer.
- * If this is false the next read on the socket will return APR_EOF.
+ * Determine whether the receive part of the socket has been closed by
+ * the peer (such that the subsequent apr_socket_read call would
+ * return APR_EOF), if the socket's read buffer is empty. This
+ * function does not block waiting for I/O.
+ *
* @param socket The socket to check
- * @remark
- * <PRE>
- * This function does not block on the socket but returns immediately in
- * any case.
- * </PRE>
+ * @param atreadeof If APR_SUCCESS is returned, *atreadeof is set to
+ * non-zero if a subsequent read will return APR_EOF
+ * @return an error is returned if it was not possible to determine the
+ * status, in which case *atreadeof is not changed.
*/
-APR_DECLARE(int) apr_socket_is_connected(apr_socket_t *socket);
+APR_DECLARE(apr_status_t) apr_socket_atreadeof(apr_socket_t *sock,
+ int *atreadeof);
/**
* 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) {
+ /* Read buffer empty -> subsequent reads would block, so,
+ * definitely not at EOF. */
+ *atreadeof = 0;
+ return APR_SUCCESS;
+ }
+ else if (rv) {
+ /* Some other error -> unexpected error. */
+ return rv;
+ }
+ else if (nfds == 1 && pfds[0].rtnevents == APR_POLLIN) {
apr_sockaddr_t unused;
apr_size_t len = 1;
- char buf[1];
- /* The socket might be closed in which case
- * the poll will return POLLIN.
- * If there is no data available the socket
- * is closed.
- */
- status = apr_socket_recvfrom(&unused, socket, MSG_PEEK,
- &buf[0], &len);
- if (status == APR_SUCCESS && len)
- return 1;
- else
- return 0;
+ char buf;
+
+ /* The socket is readable - peek to see whether it returns EOF
+ * without consuming bytes from the socket buffer. */
+ rv = apr_socket_recvfrom(&unused, sock, MSG_PEEK, &buf, &len);
+ if (rv == APR_EOF) {
+ *atreadeof = 1;
+ return APR_SUCCESS;
+ }
+ else if (rv) {
+ /* Read error -> unexpected error. */
+ return rv;
+ }
+ else {
+ *atreadeof = 0;
+ return APR_SUCCESS;
+ }
}
- else if (APR_STATUS_IS_EAGAIN(status) || APR_STATUS_IS_TIMEUP(status)) {
- return 1;
- }
- return 0;
+ /* Should not fall through here. */
+ return APR_EGENERAL;
}
Index: test/testsock.c
===================================================================
--- test/testsock.c (revision 823151)
+++ test/testsock.c (working copy)
@@ -212,6 +212,7 @@
apr_proc_t proc;
apr_size_t length = STRLEN;
char datastr[STRLEN];
+ int atreadeof = -1;
sock = setup_socket(tc);
if (!sock) return;
@@ -222,7 +223,9 @@
APR_ASSERT_SUCCESS(tc, "Problem with receiving connection", rv);
/* Check that the remote socket is still open */
- ABTS_INT_EQUAL(tc, 1, apr_socket_is_connected(sock2));
+ rv = apr_socket_atreadeof(sock2, &atreadeof);
+ APR_ASSERT_SUCCESS(tc, "Determine whether at EOF, #1", rv);
+ ABTS_INT_EQUAL(tc, atreadeof, 0);
memset(datastr, 0, STRLEN);
apr_socket_recv(sock2, datastr, &length);
@@ -232,7 +235,9 @@
ABTS_SIZE_EQUAL(tc, strlen(datastr), wait_child(tc, &proc));
/* The child is dead, so should be the remote socket */
- ABTS_INT_EQUAL(tc, 0, apr_socket_is_connected(sock2));
+ rv = apr_socket_atreadeof(sock2, &atreadeof);
+ APR_ASSERT_SUCCESS(tc, "Determine whether at EOF, #2", rv);
+ ABTS_INT_EQUAL(tc, atreadeof, 1);
rv = apr_socket_close(sock2);
APR_ASSERT_SUCCESS(tc, "Problem closing connected socket", rv);
@@ -243,7 +248,9 @@
APR_ASSERT_SUCCESS(tc, "Problem with receiving connection", rv);
/* The child closed the socket instantly */
- ABTS_INT_EQUAL(tc, 0, apr_socket_is_connected(sock2));
+ rv = apr_socket_atreadeof(sock2, &atreadeof);
+ APR_ASSERT_SUCCESS(tc, "Determine whether at EOF, #3", rv);
+ ABTS_INT_EQUAL(tc, atreadeof, 1);
wait_child(tc, &proc);
rv = apr_socket_close(sock2);
