[riot-notifications] [RIOT-OS/RIOT] net/coap: Block optimizations (#11057)

Francisco notifications at github.com
Sun Sep 29 19:49:54 CEST 2019


fjmolinas commented on this pull request.



> @@ -614,20 +614,6 @@ size_t coap_put_option(uint8_t *buf, uint16_t lastonum, uint16_t onum, const uin
     return (size_t)n;
 }
 
-size_t coap_put_option_ct(uint8_t *buf, uint16_t lastonum, uint16_t content_type)
-{
-    if (content_type == 0) {
-        return coap_put_option(buf, lastonum, COAP_OPT_CONTENT_FORMAT, NULL, 0);
-    }
-    else if (content_type <= 255) {
-        uint8_t tmp = content_type;
-        return coap_put_option(buf, lastonum, COAP_OPT_CONTENT_FORMAT, &tmp, sizeof(tmp));
-    }
-    else {
-        return coap_put_option(buf, lastonum, COAP_OPT_CONTENT_FORMAT, (uint8_t *)&content_type, sizeof(content_type));
-    }
-}
-
 static unsigned _size2szx(size_t size)

> I think the idea was to not allow arbitrary sizes to be passed to be passed around, but only coap_blksize_t that doesn't need to be converted again.
> 
> So you'd have coap_blksize_t blksize instead of size_t blksize.

Yes, that was my idea. But looking at the code again I guess doing this would mean changing the `slicer` a bit so it performs a shift on the received size. I should have though of this when reviewing #11024, but I think it is a bit out of scope for this PR, I leave it up to you @kb2ma :)

In any case the while loop could be avoided by using [`__builtin_clz`](http://www.keil.com/support/man/docs/armcc/armcc_chr1359124994619.htm) here: https://github.com/RIOT-OS/RIOT/blob/c1aa872c11f66788efa994394bbd3613e45ec5e0/sys/net/application_layer/nanocoap/nanocoap.c#L661-L664

-- 
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/11057#discussion_r329362932
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.riot-os.org/pipermail/notifications/attachments/20190929/97f00172/attachment-0001.htm>


More information about the notifications mailing list