summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorChristian Grothoff <christian@grothoff.org>2019-08-01 18:05:17 +0200
committerChristian Grothoff <christian@grothoff.org>2019-08-01 18:05:17 +0200
commitb8f3cf79d55cbf985780e6ccec0b0aec74160ce2 (patch)
tree69ba17440229fd9de30c2e95572240a3664fcc1a
parent8ad5fab2e8173476b78c3fda9a416540b925cd0f (diff)
fixes, comments, FIXMEs
-rw-r--r--src/microhttpd/mhd_send.c147
1 files changed, 94 insertions, 53 deletions
diff --git a/src/microhttpd/mhd_send.c b/src/microhttpd/mhd_send.c
index a41586d9..10cb97cb 100644
--- a/src/microhttpd/mhd_send.c
+++ b/src/microhttpd/mhd_send.c
@@ -51,8 +51,6 @@ pre_cork_setsockopt (struct MHD_Connection *connection,
#if HAVE_MSG_MORE
#else
int ret;
- const MHD_SCKT_OPT_BOOL_ off_val = 0;
- const MHD_SCKT_OPT_BOOL_ on_val = 1;
/* If sk_cork_on is already what we pass in, return. */
if (connection->sk_cork_on == want_cork)
@@ -64,33 +62,72 @@ pre_cork_setsockopt (struct MHD_Connection *connection,
ret = -1;
#if TCP_CORK
if (want_cork)
+ {
+ const MHD_SCKT_OPT_BOOL_ on_val = 1;
+
ret = setsockopt (connection->socket_fd,
IPPROTO_TCP,
TCP_CORK,
(const void *) &on_val,
sizeof (on_val));
+ }
#elif TCP_NODELAY
- ret = setsockopt (connection->socket_fd,
- IPPROTO_TCP,
- TCP_NODELAY,
- (const void *) want_cork ? &off_val : &on_val,
- sizeof (on_val));
+ {
+ const MHD_SCKT_OPT_BOOL_ off_val = 0;
+ const MHD_SCKT_OPT_BOOL_ on_val = 1;
+
+ ret = setsockopt (connection->socket_fd,
+ IPPROTO_TCP,
+ TCP_NODELAY,
+ (const void *) want_cork ? &off_val : &on_val,
+ sizeof (on_val));
+ }
#elif TCP_NOPUSH
- ret = setsockopt (connection->socket_fd,
- IPPROTO_TCP,
- TCP_NOPUSH,
- (const void *) &on_val,
- sizeof (on_val));
-#endif
+ {
+ const MHD_SCKT_OPT_BOOL_ on_val = 1;
+ // FIXME: this must be wrong, set unconditionally irrespective of 'want_cork'!
+ ret = setsockopt (connection->socket_fd,
+ IPPROTO_TCP,
+ TCP_NOPUSH,
+ (const void *) &on_val,
+ sizeof (on_val));
+ }
+#endif
if (0 == ret)
{
connection->sk_cork_on = want_cork;
}
+ else
+ {
+ switch (errno)
+ {
+ case ENOTSOCK:
+ /* FIXME: Could be we are talking to a pipe, maybe remember this
+ and avoid all setsockopt() in the future? */
+ break;
+ case EBADF:
+ /* FIXME: should we die hard here? */
+ break;
+ case EINVAL:
+ /* FIXME: optlen invalid, should at least log this, maybe die */
+ break;
+ case EFAULT:
+ /* FIXME: wopsie, should at leats log this, maybe die */
+ break;
+ case ENOPROTOOPT:
+ /* FIXME: optlen unknown, should at least log this */
+ break;
+ default:
+ /* any others? man page does not list more... */
+ break;
+ }
+ }
return;
#endif /* MSG_MORE */
}
+
/**
* Handle setsockopt calls.
*
@@ -104,8 +141,6 @@ post_cork_setsockopt (struct MHD_Connection *connection,
#if HAVE_MSG_MORE
#else
int ret;
- const MHD_SCKT_OPT_BOOL_ off_val = 0;
- const MHD_SCKT_OPT_BOOL_ on_val = 1;
/* If sk_cork_on is already what we pass in, return. */
if (connection->sk_cork_on == want_cork)
@@ -116,19 +151,29 @@ post_cork_setsockopt (struct MHD_Connection *connection,
ret = -1;
#if TCP_CORK
if (! want_cork)
+ {
+ const MHD_SCKT_OPT_BOOL_ off_val = 0;
+
ret = setsockopt (connection->socket_fd,
IPPROTO_TCP,
TCP_CORK,
&off_val,
sizeof (off_val));
+ }
#elif TCP_NODELAY
/* nothing to do */
#elif TCP_NOPUSH
- ret = setsockopt (connection->socket_fd,
- IPPROTO_TCP,
- TCP_NOPUSH,
- (const void *) &off_val,
- sizeof (off_val));
+ // FIXME: this must be wrong, set unconditionally irrespective of 'want_cork'!
+ {
+ const MHD_SCKT_OPT_BOOL_ off_val = 0;
+ const MHD_SCKT_OPT_BOOL_ on_val = 1;
+
+ ret = setsockopt (connection->socket_fd,
+ IPPROTO_TCP,
+ TCP_NOPUSH,
+ (const void *) &off_val,
+ sizeof (off_val));
+ }
#endif
if (0 == ret)
{
@@ -138,6 +183,7 @@ post_cork_setsockopt (struct MHD_Connection *connection,
#endif /* MSG_MORE */
}
+
/**
* Send buffer on connection, and remember the current state of
* the socket options; only call setsockopt when absolutely
@@ -164,11 +210,8 @@ MHD_send_on_connection_ (struct MHD_Connection *connection,
enum MHD_SendSocketOptions options)
{
bool want_cork;
- bool using_tls = false;
MHD_socket s = connection->socket_fd;
ssize_t ret;
- const MHD_SCKT_OPT_BOOL_ off_val = 0;
- const MHD_SCKT_OPT_BOOL_ on_val = 1;
/* error handling from send_param_adapter() */
if ((MHD_INVALID_SOCKET == s) || (MHD_CONNECTION_CLOSED == connection->state))
@@ -197,21 +240,15 @@ MHD_send_on_connection_ (struct MHD_Connection *connection,
break;
}
- /* ! could be avoided by redefining the variable. */
- // bool have_cork = ! connection->sk_tcp_nodelay_on;
- bool have_cork = ! connection->sk_cork_on;
-
-#ifdef HTTPS_SUPPORT
- using_tls = (0 != (connection->daemon->options & MHD_USE_TLS));
-#endif
-
#ifdef HTTPS_SUPPORT
- if (using_tls)
+ if (0 != (connection->daemon->options & MHD_USE_TLS))
{
+ bool have_cork = connection->sk_cork_on;
+
if (want_cork && ! have_cork)
{
gnutls_record_cork (connection->tls_session);
- connection->sk_cork_on = false;
+ connection->sk_cork_on = true;
}
if (buffer_size > SSIZE_MAX)
buffer_size = SSIZE_MAX;
@@ -242,7 +279,7 @@ MHD_send_on_connection_ (struct MHD_Connection *connection,
if (! want_cork && have_cork)
{
(void) gnutls_record_uncork (connection->tls_session, 0);
- connection->sk_cork_on = true;
+ connection->sk_cork_on = false;
}
}
else
@@ -285,8 +322,8 @@ MHD_send_on_connection_ (struct MHD_Connection *connection,
else if (buffer_size > (size_t) ret)
connection->epoll_state &= ~MHD_EPOLL_STATE_WRITE_READY;
#endif /* EPOLL_SUPPORT */
-
- post_cork_setsockopt (connection, want_cork);
+ if (ret == buffer_size)
+ post_cork_setsockopt (connection, want_cork);
}
return ret;
@@ -319,28 +356,26 @@ MHD_send_on_connection2_ (struct MHD_Connection *connection,
{
#if defined(HAVE_SENDMSG) || defined(HAVE_WRITEV)
MHD_socket s = connection->socket_fd;
- bool want_cork;
int iovcnt;
- int eno;
ssize_t ret;
- const MHD_SCKT_OPT_BOOL_ off_val = 0;
struct iovec vector[2];
- want_cork = ! connection->sk_cork_on;
-
- pre_cork_setsockopt (connection, want_cork);
+ /* Since we generally give the fully answer, we do not want
+ corking to happen */
+ pre_cork_setsockopt (connection, false);
vector[0].iov_base = (void *) header;
- vector[0].iov_len = strlen (header);
+ vector[0].iov_len = header_size;
vector[1].iov_base = (void *) buffer;
- vector[1].iov_len = strlen (buffer);
+ vector[1].iov_len = buffer_size;
#if HAVE_SENDMSG
- struct msghdr msg;
- memset(&msg, 0, sizeof(struct msghdr));
+ {
+ struct msghdr msg;
- msg.msg_iov = vector;
- msg.msg_iovlen = 2;
+ memset(&msg, 0, sizeof(struct msghdr));
+ msg.msg_iov = vector;
+ msg.msg_iovlen = 2;
/*
* questionable for this case, bus maybe worth considering for now:
@@ -349,21 +384,23 @@ MHD_send_on_connection2_ (struct MHD_Connection *connection,
* If you set msg_control to nonnull, NetBSD expects you to have
* msg_controllen > 0. (sys/kern/uipc_syscalls.c in do_sys_sendmsg_so)
* for reference. Thanks to pDNS (for FreeBSD), Riastradh for NetBSD.
+ * FIXME: this is unnecessary, see the memset() above?
*/
/*
msg.msg_control = 0;
msg.msg_controllen = 0;
*/
- ret = sendmsg (s, &msg, MAYBE_MSG_NOSIGNAL);
+ ret = sendmsg (s, &msg, MAYBE_MSG_NOSIGNAL);
+ }
#elif HAVE_WRITEV
iovcnt = sizeof (vector) / sizeof (struct iovec);
ret = writev (s, vector, iovcnt);
#endif
+ /* Only if we succeeded sending the full buffer, we need to make sure that
+ the OS flushes at the end */
if (ret == header_size + buffer_size)
- want_cork = false;
-
- post_cork_setsockopt (connection, want_cork);
+ post_cork_setsockopt (connection, false);
return ret;
@@ -407,6 +444,7 @@ static int freebsd_sendfile_flags_thd_p_c_;
* @param connection the MHD connection structure
* @return actual number of bytes sent
*/
+// FIXME: rename...
ssize_t
sendfile_adapter (struct MHD_Connection *connection)
{
@@ -579,7 +617,10 @@ sendfile_adapter (struct MHD_Connection *connection)
ret = (ssize_t)len;
#endif /* HAVE_FREEBSD_SENDFILE */
- post_cork_setsockopt (connection, false);
+ /* Make sure we send the data without delay ONLY if we
+ provided the complete response (not on partial write) */
+ if (ret == left)
+ post_cork_setsockopt (connection, false);
return ret;
}