[riot-notifications] [RIOT-OS/RIOT] sys/net/application_layer/nanocoap: Add path prefix option (#11098)

Ken Bannister notifications at github.com
Tue Mar 5 07:07:07 CET 2019

kb2ma requested changes on this pull request.

I have reviewed the code, and overall it looks good. Thanks for taking a comprehensive approach, and for fixing the nanocoap README!

Please also add a note to the `gcoap.h` module documentation about path matching. See the Server Operation section. Just a reference to the nanocoap documentation would be fine.

> @@ -170,6 +189,8 @@ extern "C" {
 #define COAP_POST               (0x2)
 #define COAP_PUT                (0x4)
 #define COAP_DELETE             (0x8)
+#define COAP_MATCH_SUBTREE      (0x10) /**< Path is considered as a prefix when

Use of 0x10 conflicts with the FETCH method being defined in #10193. So we also need to consider the discussion in #10214. I personally like the idea of addition of a modifier to specify the kind of lookup, but then the 'method' attribute needs to be explicitly uint32_t to accommodate it. So COAP_MATCH_SUBTREE could be 0x100.

> @@ -323,7 +323,15 @@ ssize_t coap_handle_req(coap_pkt_t *pkt, uint8_t *resp_buf, unsigned resp_buf_le
-        int res = strcmp((char *)uri, resource->path);
+        int res;

Do you mean that line 332 is a duplicate of line 329, or do you mean that the comparison in nanocoap is the same as in gcoap?

I worry about assuming that a trailing '/' means subtree lookup. For example, @haukepetersen described a [situation](https://github.com/RIOT-OS/RIOT/pull/9872#issuecomment-419366460) with a resource directory that expects an exact match on a path with a trailing '/'.

I like an explicit flag to qualify the search. Maybe we'll find other ways in the future to qualify a path in the future, like with a simple regex.

You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.riot-os.org/pipermail/notifications/attachments/20190304/90eaf3c7/attachment.html>

More information about the notifications mailing list