From 263da5196786fc50a05dcd5f4a3fbc264afa143e Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Sat, 31 Aug 2013 08:13:29 +0200 Subject: [PATCH] include: uv_udp_send{6} now takes sockaddr_in* Passing or returning structs as values makes life hard for people that work with libuv through a foreign function interface. Switch to a pointer-based approach. Fixes #684. --- include/uv-unix.h | 2 +- include/uv.h | 22 +++++--- src/unix/udp.c | 83 ++++++----------------------- src/uv-common.c | 40 +++++++++----- src/uv-common.h | 14 ++--- src/win/udp.c | 95 ++++++++++++++++------------------ test/benchmark-udp-pummel.c | 4 +- test/echo-server.c | 2 +- test/test-getsockname.c | 2 +- test/test-ref.c | 2 +- test/test-udp-dgram-too-big.c | 2 +- test/test-udp-ipv6.c | 2 +- test/test-udp-multicast-join.c | 2 +- test/test-udp-multicast-ttl.c | 2 +- test/test-udp-open.c | 2 +- test/test-udp-send-and-recv.c | 4 +- 16 files changed, 116 insertions(+), 164 deletions(-) diff --git a/include/uv-unix.h b/include/uv-unix.h index 408b44c6..e9edb11d 100644 --- a/include/uv-unix.h +++ b/include/uv-unix.h @@ -213,7 +213,7 @@ typedef struct { #define UV_UDP_SEND_PRIVATE_FIELDS \ void* queue[2]; \ struct sockaddr_in6 addr; \ - int bufcnt; \ + unsigned int nbufs; \ uv_buf_t* bufs; \ ssize_t status; \ uv_udp_send_cb send_cb; \ diff --git a/include/uv.h b/include/uv.h index 4f96ad97..393d892b 100644 --- a/include/uv.h +++ b/include/uv.h @@ -991,16 +991,19 @@ UV_EXTERN int uv_udp_set_ttl(uv_udp_t* handle, int ttl); * req UDP request handle. Need not be initialized. * handle UDP handle. Should have been initialized with `uv_udp_init`. * bufs List of buffers to send. - * bufcnt Number of buffers in `bufs`. + * nbufs Number of buffers in `bufs`. * addr Address of the remote peer. See `uv_ip4_addr`. * send_cb Callback to invoke when the data has been sent out. * * Returns: * 0 on success, or an error code < 0 on failure. */ -UV_EXTERN int uv_udp_send(uv_udp_send_t* req, uv_udp_t* handle, - uv_buf_t bufs[], int bufcnt, struct sockaddr_in addr, - uv_udp_send_cb send_cb); +UV_EXTERN int uv_udp_send(uv_udp_send_t* req, + uv_udp_t* handle, + const uv_buf_t bufs[], + unsigned int nbufs, + const struct sockaddr_in* addr, + uv_udp_send_cb send_cb); /* * Send data. If the socket has not previously been bound with `uv_udp_bind6`, @@ -1010,16 +1013,19 @@ UV_EXTERN int uv_udp_send(uv_udp_send_t* req, uv_udp_t* handle, * req UDP request handle. Need not be initialized. * handle UDP handle. Should have been initialized with `uv_udp_init`. * bufs List of buffers to send. - * bufcnt Number of buffers in `bufs`. + * nbufs Number of buffers in `bufs`. * addr Address of the remote peer. See `uv_ip6_addr`. * send_cb Callback to invoke when the data has been sent out. * * Returns: * 0 on success, or an error code < 0 on failure. */ -UV_EXTERN int uv_udp_send6(uv_udp_send_t* req, uv_udp_t* handle, - uv_buf_t bufs[], int bufcnt, struct sockaddr_in6 addr, - uv_udp_send_cb send_cb); +UV_EXTERN int uv_udp_send6(uv_udp_send_t* req, + uv_udp_t* handle, + const uv_buf_t bufs[], + unsigned int nbufs, + const struct sockaddr_in6* addr, + uv_udp_send_cb send_cb); /* * Receive data. If the socket has not previously been bound with `uv_udp_bind` diff --git a/src/unix/udp.c b/src/unix/udp.c index 6c748288..47e8b035 100644 --- a/src/unix/udp.c +++ b/src/unix/udp.c @@ -35,13 +35,6 @@ static void uv__udp_io(uv_loop_t* loop, uv__io_t* w, unsigned int revents); static void uv__udp_recvmsg(uv_loop_t* loop, uv__io_t* w, unsigned int revents); static void uv__udp_sendmsg(uv_loop_t* loop, uv__io_t* w, unsigned int revents); static int uv__udp_maybe_deferred_bind(uv_udp_t* handle, int domain); -static int uv__send(uv_udp_send_t* req, - uv_udp_t* handle, - uv_buf_t bufs[], - int bufcnt, - struct sockaddr* addr, - socklen_t addrlen, - uv_udp_send_cb send_cb); void uv__udp_close(uv_udp_t* handle) { @@ -100,8 +93,8 @@ static void uv__udp_run_pending(uv_udp_t* handle) { h.msg_name = &req->addr; h.msg_namelen = (req->addr.sin6_family == AF_INET6 ? sizeof(struct sockaddr_in6) : sizeof(struct sockaddr_in)); - h.msg_iov = (struct iovec*)req->bufs; - h.msg_iovlen = req->bufcnt; + h.msg_iov = (struct iovec*) req->bufs; + h.msg_iovlen = req->nbufs; do { size = sendmsg(handle->io_watcher.fd, &h, 0); @@ -116,19 +109,6 @@ static void uv__udp_run_pending(uv_udp_t* handle) { req->status = (size == -1 ? -errno : size); -#ifndef NDEBUG - /* Sanity check. */ - if (size != -1) { - ssize_t nbytes; - int i; - - for (nbytes = i = 0; i < req->bufcnt; i++) - nbytes += req->bufs[i].len; - - assert(size == nbytes); - } -#endif - /* Sending a datagram is an atomic operation: either all data * is written or nothing is (and EMSGSIZE is raised). That is * why we don't handle partial writes. Just pop the request @@ -400,16 +380,16 @@ static int uv__udp_maybe_deferred_bind(uv_udp_t* handle, int domain) { } -static int uv__send(uv_udp_send_t* req, - uv_udp_t* handle, - uv_buf_t bufs[], - int bufcnt, - struct sockaddr* addr, - socklen_t addrlen, - uv_udp_send_cb send_cb) { +int uv__udp_send(uv_udp_send_t* req, + uv_udp_t* handle, + const uv_buf_t bufs[], + unsigned int nbufs, + const struct sockaddr* addr, + unsigned int addrlen, + uv_udp_send_cb send_cb) { int err; - assert(bufcnt > 0); + assert(nbufs > 0); err = uv__udp_maybe_deferred_bind(handle, addr->sa_family); if (err) @@ -421,17 +401,16 @@ static int uv__send(uv_udp_send_t* req, memcpy(&req->addr, addr, addrlen); req->send_cb = send_cb; req->handle = handle; - req->bufcnt = bufcnt; + req->nbufs = nbufs; - if (bufcnt <= (int) ARRAY_SIZE(req->bufsml)) - req->bufs = req->bufsml; - else - req->bufs = malloc(bufcnt * sizeof(*bufs)); + req->bufs = req->bufsml; + if (nbufs > ARRAY_SIZE(req->bufsml)) + req->bufs = malloc(nbufs * sizeof(bufs[0])); if (req->bufs == NULL) return -ENOMEM; - memcpy(req->bufs, bufs, bufcnt * sizeof(bufs[0])); + memcpy(req->bufs, bufs, nbufs * sizeof(bufs[0])); QUEUE_INSERT_TAIL(&handle->write_queue, &req->queue); uv__io_start(handle->loop, &handle->io_watcher, UV__POLLOUT); uv__handle_start(handle); @@ -575,38 +554,6 @@ int uv_udp_getsockname(uv_udp_t* handle, struct sockaddr* name, int* namelen) { } -int uv__udp_send(uv_udp_send_t* req, - uv_udp_t* handle, - uv_buf_t bufs[], - int bufcnt, - struct sockaddr_in addr, - uv_udp_send_cb send_cb) { - return uv__send(req, - handle, - bufs, - bufcnt, - (struct sockaddr*)&addr, - sizeof addr, - send_cb); -} - - -int uv__udp_send6(uv_udp_send_t* req, - uv_udp_t* handle, - uv_buf_t bufs[], - int bufcnt, - struct sockaddr_in6 addr, - uv_udp_send_cb send_cb) { - return uv__send(req, - handle, - bufs, - bufcnt, - (struct sockaddr*)&addr, - sizeof addr, - send_cb); -} - - int uv__udp_recv_start(uv_udp_t* handle, uv_alloc_cb alloc_cb, uv_udp_recv_cb recv_cb) { diff --git a/src/uv-common.c b/src/uv-common.c index 83ee12dc..bc96029f 100644 --- a/src/uv-common.c +++ b/src/uv-common.c @@ -258,27 +258,39 @@ int uv_tcp_connect6(uv_connect_t* req, int uv_udp_send(uv_udp_send_t* req, uv_udp_t* handle, - uv_buf_t bufs[], - int bufcnt, - struct sockaddr_in addr, + const uv_buf_t bufs[], + unsigned int nbufs, + const struct sockaddr_in* addr, uv_udp_send_cb send_cb) { - if (handle->type != UV_UDP || addr.sin_family != AF_INET) - return UV_EINVAL; - else - return uv__udp_send(req, handle, bufs, bufcnt, addr, send_cb); + if (handle->type == UV_UDP && addr->sin_family == AF_INET) { + return uv__udp_send(req, + handle, + bufs, + nbufs, + (const struct sockaddr*) addr, + sizeof(*addr), + send_cb); + } + return UV_EINVAL; } int uv_udp_send6(uv_udp_send_t* req, uv_udp_t* handle, - uv_buf_t bufs[], - int bufcnt, - struct sockaddr_in6 addr, + const uv_buf_t bufs[], + unsigned int nbufs, + const struct sockaddr_in6* addr, uv_udp_send_cb send_cb) { - if (handle->type != UV_UDP || addr.sin6_family != AF_INET6) - return UV_EINVAL; - else - return uv__udp_send6(req, handle, bufs, bufcnt, addr, send_cb); + if (handle->type == UV_UDP && addr->sin6_family == AF_INET6) { + return uv__udp_send(req, + handle, + bufs, + nbufs, + (const struct sockaddr*) addr, + sizeof(*addr), + send_cb); + } + return UV_EINVAL; } diff --git a/src/uv-common.h b/src/uv-common.h index b933b98a..6fcc212a 100644 --- a/src/uv-common.h +++ b/src/uv-common.h @@ -76,18 +76,12 @@ int uv__udp_bind(uv_udp_t* handle, int uv__udp_send(uv_udp_send_t* req, uv_udp_t* handle, - uv_buf_t bufs[], - int bufcnt, - struct sockaddr_in addr, + const uv_buf_t bufs[], + unsigned int nbufs, + const struct sockaddr* addr, + unsigned int addrlen, uv_udp_send_cb send_cb); -int uv__udp_send6(uv_udp_send_t* req, - uv_udp_t* handle, - uv_buf_t bufs[], - int bufcnt, - struct sockaddr_in6 addr, - uv_udp_send_cb send_cb); - int uv__udp_recv_start(uv_udp_t* handle, uv_alloc_cb alloccb, uv_udp_recv_cb recv_cb); diff --git a/src/win/udp.c b/src/win/udp.c index c7c01159..49de701c 100644 --- a/src/win/udp.c +++ b/src/win/udp.c @@ -359,8 +359,13 @@ int uv__udp_recv_stop(uv_udp_t* handle) { } -static int uv__send(uv_udp_send_t* req, uv_udp_t* handle, uv_buf_t bufs[], - int bufcnt, struct sockaddr* addr, int addr_len, uv_udp_send_cb cb) { +static int uv__send(uv_udp_send_t* req, + uv_udp_t* handle, + const uv_buf_t bufs[], + unsigned int nbufs, + const struct sockaddr* addr, + unsigned int addrlen, + uv_udp_send_cb cb) { uv_loop_t* loop = handle->loop; DWORD result, bytes; @@ -372,11 +377,11 @@ static int uv__send(uv_udp_send_t* req, uv_udp_t* handle, uv_buf_t bufs[], result = WSASendTo(handle->socket, (WSABUF*)bufs, - bufcnt, + nbufs, &bytes, 0, addr, - addr_len, + addrlen, &req->overlapped, NULL); @@ -388,7 +393,7 @@ static int uv__send(uv_udp_send_t* req, uv_udp_t* handle, uv_buf_t bufs[], uv_insert_pending_req(loop, (uv_req_t*)req); } else if (UV_SUCCEEDED_WITH_IOCP(result == 0)) { /* Request queued by the kernel. */ - req->queued_bytes = uv_count_bufs(bufs, bufcnt); + req->queued_bytes = uv_count_bufs(bufs, nbufs); handle->reqs_pending++; REGISTER_HANDLE_REQ(loop, handle, req); } else { @@ -400,52 +405,6 @@ static int uv__send(uv_udp_send_t* req, uv_udp_t* handle, uv_buf_t bufs[], } -int uv__udp_send(uv_udp_send_t* req, uv_udp_t* handle, uv_buf_t bufs[], - int bufcnt, struct sockaddr_in addr, uv_udp_send_cb cb) { - int err; - - if (!(handle->flags & UV_HANDLE_BOUND)) { - err = uv_udp_try_bind(handle, - (const struct sockaddr*) &uv_addr_ip4_any_, - sizeof(uv_addr_ip4_any_), - 0); - if (err) - return err; - } - - return uv__send(req, - handle, - bufs, - bufcnt, - (struct sockaddr*) &addr, - sizeof addr, - cb); -} - - -int uv__udp_send6(uv_udp_send_t* req, uv_udp_t* handle, uv_buf_t bufs[], - int bufcnt, struct sockaddr_in6 addr, uv_udp_send_cb cb) { - int err; - - if (!(handle->flags & UV_HANDLE_BOUND)) { - err = uv_udp_try_bind(handle, - (const struct sockaddr*) &uv_addr_ip6_any_, - sizeof(uv_addr_ip6_any_), - 0); - if (err) - return err; - } - - return uv__send(req, - handle, - bufs, - bufcnt, - (struct sockaddr*) &addr, - sizeof addr, - cb); -} - - void uv_process_udp_recv_req(uv_loop_t* loop, uv_udp_t* handle, uv_req_t* req) { uv_buf_t buf; @@ -753,3 +712,37 @@ int uv__udp_bind(uv_udp_t* handle, return 0; } + + +/* This function is an egress point, i.e. it returns libuv errors rather than + * system errors. + */ +int uv__udp_send(uv_udp_send_t* req, + uv_udp_t* handle, + const uv_buf_t bufs[], + unsigned int nbufs, + const struct sockaddr* addr, + unsigned int addrlen, + uv_udp_send_cb send_cb) { + const struct sockaddr* bind_addr; + int err; + + if (!(handle->flags & UV_HANDLE_BOUND)) { + if (addrlen == sizeof(uv_addr_ip4_any_)) { + bind_addr = (const struct sockaddr*) &uv_addr_ip4_any_; + } else if (addrlen == sizeof(uv_addr_ip6_any_)) { + bind_addr = (const struct sockaddr*) &uv_addr_ip6_any_; + } else { + abort(); + } + err = uv_udp_try_bind(handle, bind_addr, addrlen, 0); + if (err) + return uv_translate_sys_error(err); + } + + err = uv__send(req, handle, bufs, nbufs, addr, addrlen, send_cb); + if (err) + return uv_translate_sys_error(err); + + return 0; +} diff --git a/test/benchmark-udp-pummel.c b/test/benchmark-udp-pummel.c index ce66600f..3dec2631 100644 --- a/test/benchmark-udp-pummel.c +++ b/test/benchmark-udp-pummel.c @@ -100,7 +100,7 @@ send: &s->udp_handle, bufs, ARRAY_SIZE(bufs), - s->addr, + &s->addr, send_cb)); send_cb_called++; } @@ -195,7 +195,7 @@ static int pummel(unsigned int n_senders, &s->udp_handle, bufs, ARRAY_SIZE(bufs), - s->addr, + &s->addr, send_cb)); } diff --git a/test/echo-server.c b/test/echo-server.c index 160a85cf..69742019 100644 --- a/test/echo-server.c +++ b/test/echo-server.c @@ -208,7 +208,7 @@ static void on_recv(uv_udp_t* handle, handle, &sndbuf, 1, - *(const struct sockaddr_in*) addr, + (const struct sockaddr_in*) addr, on_send); ASSERT(r == 0); } diff --git a/test/test-getsockname.c b/test/test-getsockname.c index 57d276f2..87bb616d 100644 --- a/test/test-getsockname.c +++ b/test/test-getsockname.c @@ -310,7 +310,7 @@ static void udp_sender(void) { buf = uv_buf_init("PING", 4); ASSERT(0 == uv_ip4_addr("127.0.0.1", server_port, &server_addr)); - r = uv_udp_send(&send_req, &udp, &buf, 1, server_addr, udp_send); + r = uv_udp_send(&send_req, &udp, &buf, 1, &server_addr, udp_send); ASSERT(!r); } diff --git a/test/test-ref.c b/test/test-ref.c index aea1244e..27289144 100644 --- a/test/test-ref.c +++ b/test/test-ref.c @@ -320,7 +320,7 @@ TEST_IMPL(udp_ref3) { ASSERT(0 == uv_ip4_addr("127.0.0.1", TEST_PORT, &addr)); uv_udp_init(uv_default_loop(), &h); - uv_udp_send(&req, &h, &buf, 1, addr, (uv_udp_send_cb)req_cb); + uv_udp_send(&req, &h, &buf, 1, &addr, (uv_udp_send_cb)req_cb); uv_unref((uv_handle_t*)&h); uv_run(uv_default_loop(), UV_RUN_DEFAULT); ASSERT(req_cb_called == 1); diff --git a/test/test-udp-dgram-too-big.c b/test/test-udp-dgram-too-big.c index ef08af27..ecbafa96 100644 --- a/test/test-udp-dgram-too-big.c +++ b/test/test-udp-dgram-too-big.c @@ -70,7 +70,7 @@ TEST_IMPL(udp_dgram_too_big) { buf = uv_buf_init(dgram, sizeof dgram); ASSERT(0 == uv_ip4_addr("127.0.0.1", TEST_PORT, &addr)); - r = uv_udp_send(&req_, &handle_, &buf, 1, addr, send_cb); + r = uv_udp_send(&req_, &handle_, &buf, 1, &addr, send_cb); ASSERT(r == 0); ASSERT(close_cb_called == 0); diff --git a/test/test-udp-ipv6.c b/test/test-udp-ipv6.c index 1f501a23..f545f81b 100644 --- a/test/test-udp-ipv6.c +++ b/test/test-udp-ipv6.c @@ -120,7 +120,7 @@ static void do_test(uv_udp_recv_cb recv_cb, int bind_flags) { buf = uv_buf_init("PING", 4); ASSERT(0 == uv_ip4_addr("127.0.0.1", TEST_PORT, &addr)); - r = uv_udp_send(&req_, &client, &buf, 1, addr, send_cb); + r = uv_udp_send(&req_, &client, &buf, 1, &addr, send_cb); ASSERT(r == 0); r = uv_timer_init(uv_default_loop(), &timeout); diff --git a/test/test-udp-multicast-join.c b/test/test-udp-multicast-join.c index 3b162e24..0e750d84 100644 --- a/test/test-udp-multicast-join.c +++ b/test/test-udp-multicast-join.c @@ -124,7 +124,7 @@ TEST_IMPL(udp_multicast_join) { buf = uv_buf_init("PING", 4); /* server sends "PING" */ - r = uv_udp_send(&req, &server, &buf, 1, addr, sv_send_cb); + r = uv_udp_send(&req, &server, &buf, 1, &addr, sv_send_cb); ASSERT(r == 0); ASSERT(close_cb_called == 0); diff --git a/test/test-udp-multicast-ttl.c b/test/test-udp-multicast-ttl.c index 194ff90f..390ea579 100644 --- a/test/test-udp-multicast-ttl.c +++ b/test/test-udp-multicast-ttl.c @@ -72,7 +72,7 @@ TEST_IMPL(udp_multicast_ttl) { /* server sends "PING" */ buf = uv_buf_init("PING", 4); ASSERT(0 == uv_ip4_addr("239.255.0.1", TEST_PORT, &addr)); - r = uv_udp_send(&req, &server, &buf, 1, addr, sv_send_cb); + r = uv_udp_send(&req, &server, &buf, 1, &addr, sv_send_cb); ASSERT(r == 0); ASSERT(close_cb_called == 0); diff --git a/test/test-udp-open.c b/test/test-udp-open.c index 3c554e47..3d64b00f 100644 --- a/test/test-udp-open.c +++ b/test/test-udp-open.c @@ -146,7 +146,7 @@ TEST_IMPL(udp_open) { r = uv_udp_recv_start(&client, alloc_cb, recv_cb); ASSERT(r == 0); - r = uv_udp_send(&send_req, &client, &buf, 1, addr, send_cb); + r = uv_udp_send(&send_req, &client, &buf, 1, &addr, send_cb); ASSERT(r == 0); uv_run(uv_default_loop(), UV_RUN_DEFAULT); diff --git a/test/test-udp-send-and-recv.c b/test/test-udp-send-and-recv.c index db31937e..d8da37b7 100644 --- a/test/test-udp-send-and-recv.c +++ b/test/test-udp-send-and-recv.c @@ -156,7 +156,7 @@ static void sv_recv_cb(uv_udp_t* handle, handle, &sndbuf, 1, - *(const struct sockaddr_in*) addr, + (const struct sockaddr_in*) addr, sv_send_cb); ASSERT(r == 0); @@ -189,7 +189,7 @@ TEST_IMPL(udp_send_and_recv) { /* client sends "PING", expects "PONG" */ buf = uv_buf_init("PING", 4); - r = uv_udp_send(&req, &client, &buf, 1, addr, cl_send_cb); + r = uv_udp_send(&req, &client, &buf, 1, &addr, cl_send_cb); ASSERT(r == 0); ASSERT(close_cb_called == 0);