From e3db58f013dddf98ffd7ffeb93f7fe3bdca68577 Mon Sep 17 00:00:00 2001 From: chrysn Date: Fri, 2 Oct 2020 20:51:40 +0200 Subject: [PATCH 1/3] net/gcoap: Avoid endless loop on error The NULL case can not regularly be reached (because regularly gcoap_register_listener sets thel link_encoder to a default one), but if it is (eg. because an application unsets its link_encoder to hide a resource set at runtime), the existing `continue` is a good idea (skip over this entry) but erroneously created an endless loop by skipping the advancement step. --- sys/net/application_layer/gcoap/gcoap.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/sys/net/application_layer/gcoap/gcoap.c b/sys/net/application_layer/gcoap/gcoap.c index 7dcb442da8..f745f519b0 100644 --- a/sys/net/application_layer/gcoap/gcoap.c +++ b/sys/net/application_layer/gcoap/gcoap.c @@ -887,7 +887,7 @@ int gcoap_get_resource_list(void *buf, size_t maxlen, uint8_t cf) ctx.flags = COAP_LINK_FLAG_INIT_RESLIST; /* write payload */ - while (listener) { + for (; listener != NULL; listener = listener->next) { if (!listener->link_encoder) { continue; } @@ -912,8 +912,6 @@ int gcoap_get_resource_list(void *buf, size_t maxlen, uint8_t cf) break; } } - - listener = listener->next; } return (int)pos; From 6350d22bc9cc9d49aa226f023d65bdcb0f73c41c Mon Sep 17 00:00:00 2001 From: chrysn Date: Fri, 2 Oct 2020 21:00:40 +0200 Subject: [PATCH 2/3] net/gcoap: Register additional resources head-first This simplifies (written and compiled) code by doing a head rather than a tail insertion of the new listener into gcoap's list. As handling of listeners without a link_encoder is now fixed, gcoap_get_resource_list can handles this now without having to manually skip over the .well-known/core handler (which is not the first entry any more now). Incidentally, this allows the user to install a custom handler for .well-known/core, as the default handler is now evaluated last. --- sys/net/application_layer/gcoap/gcoap.c | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/sys/net/application_layer/gcoap/gcoap.c b/sys/net/application_layer/gcoap/gcoap.c index f745f519b0..56d8d6d5d6 100644 --- a/sys/net/application_layer/gcoap/gcoap.c +++ b/sys/net/application_layer/gcoap/gcoap.c @@ -636,17 +636,12 @@ kernel_pid_t gcoap_init(void) void gcoap_register_listener(gcoap_listener_t *listener) { - /* Add the listener to the end of the linked list. */ - gcoap_listener_t *_last = _coap_state.listeners; - while (_last->next) { - _last = _last->next; - } - - listener->next = NULL; if (!listener->link_encoder) { listener->link_encoder = gcoap_encode_link; } - _last->next = listener; + + listener->next = _coap_state.listeners; + _coap_state.listeners = listener; } int gcoap_req_init(coap_pkt_t *pdu, uint8_t *buf, size_t len, @@ -875,8 +870,7 @@ int gcoap_get_resource_list(void *buf, size_t maxlen, uint8_t cf) { assert(cf == COAP_FORMAT_LINK); - /* skip the first listener, gcoap itself (we skip /.well-known/core) */ - gcoap_listener_t *listener = _coap_state.listeners->next; + gcoap_listener_t *listener = _coap_state.listeners; char *out = (char *)buf; size_t pos = 0; From e2e56cce21527b81f8058418957bcd3108f44986 Mon Sep 17 00:00:00 2001 From: chrysn Date: Sat, 3 Oct 2020 00:00:46 +0200 Subject: [PATCH 3/3] tests/unittests: Change sequence of expected resources This is a consequence of "Register additional resources head-first", and is acceptable as there are no semantics to the link-format message. --- tests/unittests/tests-gcoap/tests-gcoap.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unittests/tests-gcoap/tests-gcoap.c b/tests/unittests/tests-gcoap/tests-gcoap.c index 7ba737868d..71c6464574 100644 --- a/tests/unittests/tests-gcoap/tests-gcoap.c +++ b/tests/unittests/tests-gcoap/tests-gcoap.c @@ -49,7 +49,7 @@ static gcoap_listener_t listener_second = { .next = NULL }; -static const char *resource_list_str = ",,,"; +static const char *resource_list_str = ",,,"; /* * Client GET request success case. Test request generation.