[riot-notifications] [RIOT-OS/RIOT] gcoap: DEVELHELP checks resources are sorted (#7580)

Sebastian Meiling notifications at github.com
Mon Sep 18 11:51:51 CEST 2017


smlng requested changes on this pull request.



> @@ -570,6 +570,32 @@ static void _find_obs_memo_resource(gcoap_observe_memo_t **memo,
 }
 
 /*
+ * Check listener structure
+ *
+ *  - resources should be sorted alphabetically
+ */
+#ifdef DEVELHELP
+static void _check_listener(gcoap_listener_t *listener)

IMHO a check function should have return value, i.e., `0` on success and anything else on error.

> @@ -570,6 +570,32 @@ static void _find_obs_memo_resource(gcoap_observe_memo_t **memo,
 }
 
 /*
+ * Check listener structure
+ *
+ *  - resources should be sorted alphabetically
+ */
+#ifdef DEVELHELP
+static void _check_listener(gcoap_listener_t *listener)
+{
+    const char *prev_path = "";
+    const char *path = NULL;
+
+    for (size_t i = 0; i < listener->resources_len; i++) {

Why not start with `i = 1` and use `listener->resources[i-1]->path` and `listener->resources[i]->path` in `strcmp` that way omitting these extra variables plus remembering the previous path. 

> @@ -594,6 +620,8 @@ kernel_pid_t gcoap_init(void)
 
 void gcoap_register_listener(gcoap_listener_t *listener)
 {
+    _check_listener(listener);

if (as suggested above) check_listener would have a return value, you could use `assert(!_check_listener(listener);` here.

> @@ -806,7 +834,7 @@ uint8_t gcoap_op_state(void)
 int gcoap_get_resource_list(void *buf, size_t maxlen, uint8_t cf)
 {
     assert(cf == COAP_CT_LINK_FORMAT);
-#ifndef DEVELHELP
+#ifdef NDEBUG

why this change?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/RIOT-OS/RIOT/pull/7580#pullrequestreview-63299941
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.riot-os.org/pipermail/notifications/attachments/20170918/7e90e3cb/attachment.html>


More information about the notifications mailing list