Mercurial > louis > mq > lightsd
changeset 251:0d981a9b602e
Finished fixes
author | Louis Opter <kalessin@kalessin.fr> |
---|---|
date | Sat, 15 Aug 2015 16:37:19 -0700 |
parents | a0066335c86e |
children | 953786a62e00 |
files | fix_command_pipe_read_loop.patch fix_ipv4_address_formatting.patch series use_jq_when_avalaible.patch |
diffstat | 4 files changed, 0 insertions(+), 601 deletions(-) [+] |
line wrap: on
line diff
--- a/fix_command_pipe_read_loop.patch Sat Aug 15 16:36:28 2015 -0700 +++ /dev/null Thu Jan 01 00:00:00 1970 +0000 @@ -1,487 +0,0 @@ -# HG changeset patch -# Parent 870e94c0386d9595949a21c3486875759c4a1ff6 -Fix infinite loop in the command pipe read callback - -Looks like I suck at Unix, EOF & EAGAIN weren't handled properly. The -pipe is now re-opened on EOF. - -We could probably make the command pipe bi-directional at this point. - -I wish conky wasn't fucked nowadays, that would have let me spot this -earlier. - -diff --git a/core/pipe.c b/core/pipe.c ---- a/core/pipe.c -+++ b/core/pipe.c -@@ -41,7 +41,7 @@ - SLIST_HEAD_INITIALIZER(&lgtd_command_pipes); - - static void --lgtd_command_pipe_close(struct lgtd_command_pipe *pipe) -+_lgtd_command_pipe_close(struct lgtd_command_pipe *pipe) - { - assert(pipe); - -@@ -49,16 +49,24 @@ - if (pipe->fd != -1) { - close(pipe->fd); - } -- unlink(pipe->path); - SLIST_REMOVE(&lgtd_command_pipes, pipe, lgtd_command_pipe, link); - evbuffer_free(pipe->read_buf); - event_free(pipe->read_ev); -- -- lgtd_info("closed command pipe %s", pipe->path); - free(pipe); - } - - static void -+lgtd_command_pipe_close(struct lgtd_command_pipe *pipe) -+{ -+ const char *path = pipe->path; -+ _lgtd_command_pipe_close(pipe); -+ unlink(path); -+ lgtd_info("closed command pipe %s", path); -+} -+ -+static void lgtd_command_pipe_reset(struct lgtd_command_pipe *); -+ -+static void - lgtd_command_pipe_read_callback(evutil_socket_t socket, short events, void *ctx) - { - assert(ctx); -@@ -70,7 +78,6 @@ - struct lgtd_command_pipe *pipe = ctx; - - bool drain = false; -- int read = 0; - for (int nbytes = evbuffer_read(pipe->read_buf, pipe->fd, -1); - nbytes; - nbytes = evbuffer_read(pipe->read_buf, pipe->fd, -1)) { -@@ -80,16 +87,12 @@ - } - if (errno != EAGAIN) { - lgtd_warn("read error on command pipe %s", pipe->path); -- const char *path = pipe->path; -- lgtd_command_pipe_close(pipe); -- lgtd_command_pipe_open(path); -- return; -+ break; - } -- continue; -+ return; // EAGAIN, go back to the event loop - } - - if (!drain) { -- read += nbytes; - next_request: - (void)0; - const char *buf = (char *)evbuffer_pullup(pipe->read_buf, -1); -@@ -127,25 +130,22 @@ - jsmn_init(&pipe->client.jsmn_ctx); - int request_size = pipe->client.jsmn_tokens[0].end; - evbuffer_drain(pipe->read_buf, request_size); -- read -= request_size; -- if (read) { -+ if (request_size < bufsz) { - goto next_request; - } - break; - } -- } else { -- evbuffer_drain(pipe->read_buf, read + nbytes); -- read = 0; -+ } -+ -+ if (drain) { -+ ssize_t bufsz = evbuffer_get_length(pipe->read_buf); -+ evbuffer_drain(pipe->read_buf, bufsz); -+ drain = false; -+ jsmn_init(&pipe->client.jsmn_ctx); - } - } - -- if (read) { -- lgtd_debug( -- "pipe %s: discarding %d bytes of unusable data", pipe->path, read -- ); -- evbuffer_drain(pipe->read_buf, read); -- } -- jsmn_init(&pipe->client.jsmn_ctx); -+ lgtd_command_pipe_reset(pipe); - } - - static mode_t -@@ -156,8 +156,8 @@ - return mask; - } - --bool --lgtd_command_pipe_open(const char *path) -+static bool -+_lgtd_command_pipe_open(const char *path) - { - assert(path); - -@@ -218,8 +218,6 @@ - goto error; - } - -- lgtd_info("command pipe ready at %s", pipe->path); -- - SLIST_INSERT_HEAD(&lgtd_command_pipes, pipe, link); - - return true; -@@ -239,6 +237,27 @@ - return false; - } - -+static void -+lgtd_command_pipe_reset(struct lgtd_command_pipe *pipe) -+{ -+ const char *path = pipe->path; -+ // we could optimize a bit to avoid re-allocations here: -+ _lgtd_command_pipe_close(pipe); -+ if (!_lgtd_command_pipe_open(path)) { -+ lgtd_warn("can't re-open pipe %s", path); -+ } -+} -+ -+bool -+lgtd_command_pipe_open(const char *path) -+{ -+ if (_lgtd_command_pipe_open(path)) { -+ lgtd_info("command pipe ready at %s", path); -+ return true; -+ } -+ return false; -+} -+ - void - lgtd_command_pipe_close_all(void) - { -diff --git a/tests/core/pipe/test_pipe_read_callback.c b/tests/core/pipe/test_pipe_read_callback.c ---- a/tests/core/pipe/test_pipe_read_callback.c -+++ b/tests/core/pipe/test_pipe_read_callback.c -@@ -7,6 +7,7 @@ - #include "lifx/wire_proto.h" - - #define MOCKED_EVENT_NEW -+#define MOCKED_EVENT_DEL - #define MOCKED_EVBUFFER_NEW - #define MOCKED_EVBUFFER_READ - #define MOCKED_EVBUFFER_PULLUP -@@ -69,6 +70,16 @@ - return (void *)1; - } - -+static int event_del_call_count = 0; -+ -+int -+event_del(struct event *ev) -+{ -+ (void)ev; -+ event_del_call_count++; -+ return 0; -+} -+ - static int - get_nbytes_read(int call_count) - { -@@ -186,8 +197,22 @@ - } - - struct lgtd_command_pipe *pipe = SLIST_FIRST(&lgtd_command_pipes); -+ lgtd_command_pipe_read_callback(pipe->fd, EV_READ, pipe); -+ if (event_del_call_count != 1) { -+ errx(1, "the pipe wasn't reset"); -+ } - -+ jsonrpc_dispatch_request_call_count = 0; -+ evbuffer_drain_call_count = 0; -+ evbuffer_read_call_count = 0; -+ evbuffer_pullup_call_count = 0; -+ evbuffer_get_length_call_count = 0; -+ event_del_call_count = 0; -+ pipe = SLIST_FIRST(&lgtd_command_pipes); - lgtd_command_pipe_read_callback(pipe->fd, EV_READ, pipe); -+ if (event_del_call_count != 1) { -+ errx(1, "the pipe wasn't reset"); -+ } - - return 0; - } -diff --git a/tests/core/pipe/test_pipe_read_callback_yield_on_eagain.c b/tests/core/pipe/test_pipe_read_callback_yield_on_eagain.c -new file mode 100644 ---- /dev/null -+++ b/tests/core/pipe/test_pipe_read_callback_yield_on_eagain.c -@@ -0,0 +1,269 @@ -+#include "pipe.c" -+ -+#include <sys/tree.h> -+#include <endian.h> -+#include <limits.h> -+ -+#include "lifx/wire_proto.h" -+ -+#define MOCKED_EVENT_NEW -+#define MOCKED_EVBUFFER_NEW -+#define MOCKED_EVBUFFER_READ -+#define MOCKED_EVBUFFER_PULLUP -+#define MOCKED_EVBUFFER_GET_LENGTH -+#define MOCKED_EVBUFFER_DRAIN -+#include "mock_event2.h" -+#include "mock_gateway.h" -+ -+#include "tests_utils.h" -+#define MOCKED_JSONRPC_DISPATCH_REQUEST -+#include "tests_pipe_utils.h" -+ -+#define REQUEST_1 "{" \ -+ "\"jsonrpc\": \"2.0\"," \ -+ "\"method\": \"get_light_state\"," \ -+ "\"params\": [\"*\"]," \ -+ "\"id\": 42" \ -+"}" -+ -+#define REQUEST_2 "{" \ -+ "\"jsonrpc\": \"2.0\"," \ -+ "\"method\": \"power_on\"," \ -+ "\"params\": [\"*\"]," \ -+ "\"id\": 43" \ -+"}" -+ -+static unsigned char request[] = ( -+ REQUEST_1 -+ REQUEST_2 -+); -+ -+static char *tmpdir = NULL; -+ -+void -+cleanup_tmpdir(void) -+{ -+ lgtd_tests_remove_temp_dir(tmpdir); -+} -+ -+static int jsonrpc_dispatch_request_call_count = 0; -+ -+void -+lgtd_jsonrpc_dispatch_request(struct lgtd_client *client, int parsed) -+{ -+ (void)client; -+ (void)parsed; -+ -+ if (!parsed) { -+ errx(1, "number of parsed json tokens not passed in"); -+ } -+ -+ switch (jsonrpc_dispatch_request_call_count) { -+ case 0: -+ if (memcmp(client->json, request, sizeof(request) - 1)) { -+ errx( -+ 1, "got unexpected json %s (expected %s)", -+ client->json, request -+ ); -+ } -+ break; -+ case 1: -+ if (memcmp(client->json, REQUEST_2, sizeof(REQUEST_2) - 1)) { -+ errx( -+ 1, "got unexpected json %s (expected %s)", -+ client->json, REQUEST_2 -+ ); -+ } -+ break; -+ default: -+ errx( -+ 1, "jsonrpc_dispatch_request_call_count = %d", -+ jsonrpc_dispatch_request_call_count -+ ); -+ break; -+ } -+ -+ jsonrpc_dispatch_request_call_count++; -+} -+ -+struct event * -+event_new(struct event_base *base, -+ evutil_socket_t fd, -+ short events, -+ event_callback_fn cb, -+ void *ctx) -+{ -+ (void)base; -+ (void)fd; -+ (void)events; -+ (void)cb; -+ (void)ctx; -+ -+ return (void *)1; -+} -+ -+struct evbuffer * -+evbuffer_new(void) -+{ -+ return (void *)2; -+} -+ -+static int evbuffer_drain_call_count = 0; -+ -+int -+evbuffer_drain(struct evbuffer *buf, size_t len) -+{ -+ if (buf != (void *)2) { -+ errx(1, "got unexpected buf %p (expected %p)", buf, (void *)2); -+ } -+ -+ jsmn_parser jsmn_ctx; -+ jsmn_init(&jsmn_ctx); -+ struct lgtd_command_pipe *pipe = SLIST_FIRST(&lgtd_command_pipes); -+ if (memcmp(&pipe->client.jsmn_ctx, &jsmn_ctx, sizeof(jsmn_ctx))) { -+ errx(1, "the client json parser context wasn't re-initialized"); -+ } -+ -+ switch (evbuffer_drain_call_count) { -+ case 0: -+ if (len != sizeof(REQUEST_1) - 1) { -+ errx( -+ 1, "trying to drain %ju bytes (expected %ju)", -+ (uintmax_t)len, (uintmax_t)sizeof(REQUEST_1) - 1 -+ ); -+ } -+ break; -+ case 1: -+ if (len != sizeof(REQUEST_2) - 1) { -+ errx( -+ 1, "trying to drain %ju bytes (expected %ju)", -+ (uintmax_t)len, (uintmax_t)sizeof(REQUEST_2) - 1 -+ ); -+ } -+ break; -+ default: -+ errx(1, "evbuffer_drain_call_count = %d", evbuffer_drain_call_count); -+ break; -+ } -+ evbuffer_drain_call_count++; -+ -+ return 0; -+} -+ -+static int evbuffer_pullup_call_count = 0; -+ -+unsigned char * -+evbuffer_pullup(struct evbuffer *buf, ev_ssize_t size) -+{ -+ if (buf != (void *)2) { -+ errx(1, "got unexpected buf %p (expected %p)", buf, (void *)2); -+ } -+ -+ if (size != -1) { -+ errx( -+ 1, "got unexpected size %jd in pullup (expected -1)", (intmax_t)size -+ ); -+ } -+ -+ int offset; -+ switch (evbuffer_pullup_call_count) { -+ case 0: -+ offset = 0; -+ break; -+ case 1: -+ offset = sizeof(REQUEST_1) - 1; -+ break; -+ default: -+ offset = sizeof(request); -+ break; -+ } -+ evbuffer_pullup_call_count++; -+ -+ return &request[offset]; -+} -+ -+static int evbuffer_get_length_call_count = 0; -+ -+size_t -+evbuffer_get_length(const struct evbuffer *buf) -+{ -+ if (buf != (void *)2) { -+ errx(1, "got unexpected buf %p (expected %p)", buf, (void *)2); -+ } -+ -+ size_t len; -+ switch (evbuffer_get_length_call_count) { -+ case 0: -+ len = sizeof(REQUEST_1) - 1; -+ break; -+ case 1: -+ len = sizeof(request) - sizeof(REQUEST_1); -+ break; -+ default: -+ len = 0; -+ break; -+ } -+ evbuffer_get_length_call_count++; -+ -+ return len; -+} -+ -+static int evbuffer_read_call_count = 0; -+ -+int -+evbuffer_read(struct evbuffer *buf, evutil_socket_t fd, int howmuch) -+{ -+ if (buf != (void *)2) { -+ errx(1, "got unexpected buf %p (expected %p)", buf, (void *)2); -+ } -+ -+ struct lgtd_command_pipe *pipe = SLIST_FIRST(&lgtd_command_pipes); -+ if (fd != pipe->fd) { -+ errx(1, "got unexpected fd %d (expected %d)", fd, pipe->fd); -+ } -+ -+ if (howmuch != -1) { -+ errx( -+ 1, "got unexpected howmuch bytes to read %d (expected -1)", howmuch -+ ); -+ } -+ -+ int rv = 0; -+ switch (evbuffer_read_call_count) { -+ case 0: -+ rv = sizeof(REQUEST_1) - 1; -+ break; -+ case 1: -+ rv = -1; -+ errno = EAGAIN; -+ break; -+ case 2: -+ rv = sizeof(request) - sizeof(REQUEST_1); -+ break; -+ default: -+ break; -+ } -+ evbuffer_read_call_count++; -+ -+ return rv; -+} -+ -+int -+main(void) -+{ -+ tmpdir = lgtd_tests_make_temp_dir(); -+ atexit(cleanup_tmpdir); -+ -+ char path[PATH_MAX] = { 0 }; -+ snprintf(path, sizeof(path), "%s/lightsd.pipe", tmpdir); -+ if (!lgtd_command_pipe_open(path)) { -+ errx(1, "couldn't open pipe"); -+ } -+ -+ struct lgtd_command_pipe *pipe = SLIST_FIRST(&lgtd_command_pipes); -+ -+ lgtd_command_pipe_read_callback(pipe->fd, EV_READ, pipe); -+ lgtd_command_pipe_read_callback(pipe->fd, EV_READ, pipe); -+ -+ return 0; -+}
--- a/fix_ipv4_address_formatting.patch Sat Aug 15 16:36:28 2015 -0700 +++ /dev/null Thu Jan 01 00:00:00 1970 +0000 @@ -1,54 +0,0 @@ -# HG changeset patch -# Parent 118f1bcc0f067c493b1f5e112c02ac92fd922dc1 -Fix IPv4 address formatting - -diff --git a/core/lightsd.h b/core/lightsd.h ---- a/core/lightsd.h -+++ b/core/lightsd.h -@@ -28,6 +28,10 @@ - .tv_sec = (v) / 1000, \ - .tv_usec = ((v) % 1000) * 1000 \ - } -+#define LGTD_SNPRINTF_APPEND(buf, i, bufsz, ...) do { \ -+ int n = snprintf(&(buf)[(i)], bufsz - i, __VA_ARGS__); \ -+ (i) = LGTD_MIN((i) + n, bufsz); \ -+} while (0) - - enum lgtd_verbosity { - LGTD_DEBUG = 0, -diff --git a/core/log.c b/core/log.c ---- a/core/log.c -+++ b/core/log.c -@@ -26,6 +26,7 @@ - #include <stdbool.h> - #include <stdint.h> - #include <stdio.h> -+#include <stdlib.h> - #include <time.h> - - #if LGTD_HAVE_LIBBSD -@@ -100,12 +101,22 @@ - assert(buf); - assert(buflen > 0); - -+ const char *printed; - if (peer->ss_family == AF_INET) { - const struct sockaddr_in *in_peer = (const struct sockaddr_in *)peer; -- inet_ntop(AF_INET, &in_peer->sin_addr, buf, buflen); -+ int i = 0; -+ LGTD_SNPRINTF_APPEND(buf, i, buflen, "::ffff:"); -+ printed = inet_ntop(AF_INET, &in_peer->sin_addr, &buf[i], buflen - i); - } else { - const struct sockaddr_in6 *in6_peer = (const struct sockaddr_in6 *)peer; -- inet_ntop(AF_INET6, &in6_peer->sin6_addr, buf, buflen); -+ printed = inet_ntop(AF_INET6, &in6_peer->sin6_addr, buf, buflen); -+ } -+ if (!printed) { -+ buf[0] = 0; -+ lgtd_warnx("not enough space to log an ip address"); -+#ifndef NDEBUG -+ abort(); -+#endif - } - } -
--- a/series Sat Aug 15 16:36:28 2015 -0700 +++ b/series Sat Aug 15 16:37:19 2015 -0700 @@ -1,6 +1,3 @@ -use_jq_when_avalaible.patch -fix_ipv4_address_formatting.patch -fix_command_pipe_read_loop.patch add_start_stop_timer.patch implement_some_metadata_packet_types.patch blublu
--- a/use_jq_when_avalaible.patch Sat Aug 15 16:36:28 2015 -0700 +++ /dev/null Thu Jan 01 00:00:00 1970 +0000 @@ -1,57 +0,0 @@ -# HG changeset patch -# Parent dfe2227266b4be40a52a4e2cfa88b0e9e8c47523 -Use jq when available in lightsc.sh - -diff --git a/examples/lightsc.sh b/examples/lightsc.sh ---- a/examples/lightsc.sh -+++ b/examples/lightsc.sh -@@ -24,7 +24,7 @@ - # tags and labels into double quotes '"likethis"'. Also keep in mind - # that the pipe is write-only you cannot read any result back. - --_b64e() { -+_lightsc_b64e() { - if type base64 >/dev/null 2>&1 ; then - base64 - elif type b64encode >/dev/null 2>&1 ; then -@@ -35,14 +35,22 @@ - fi - } - --_gen_request_id() { -+_lightsc_gen_request_id() { - if type dd >/dev/null 2>&1 ; then -- printf '"%s"' `dd if=/dev/urandom bs=8 count=1 2>&- | _b64e` -+ printf '"%s"' `dd if=/dev/urandom bs=8 count=1 2>&- | _lightsc_b64e` - else - echo null - fi - } - -+_lightsc_jq() { -+ if type jq >/dev/null 2>&1 ; then -+ jq . -+ else -+ cat -+ fi -+} -+ - lightsc() { - if [ $# -lt 2 ] ; then - echo >&2 "Usage: $0 METHOD PARAMS ..." -@@ -61,12 +69,13 @@ - params=$params,$target - done - -- tee $pipe <<EOF -+ tee $pipe <<EOF | - { - "jsonrpc": "2.0", - "method": "$method", - "params": [$params], -- "id": `_gen_request_id` -+ "id": `_lightsc_gen_request_id` - } - EOF -+_lightsc_jq - }