[riot-notifications] [RIOT-OS/RIOT] sys/tsrb: bug fix for tsrb_get_one() (#11624)

Martine Lenders notifications at github.com
Wed Jun 5 11:22:56 CEST 2019


miri64 commented on this pull request.



> @@ -32,7 +32,7 @@ static char _pop(tsrb_t *rb)
 int tsrb_get_one(tsrb_t *rb)
 {
     if (!tsrb_empty(rb)) {
-        return _pop(rb);
+        return (unsigned char)_pop(rb);

> this still looks wrong to me bc it uses `char` not `unsigned char` when adding stuff also the buffer is `char`. It just does not look really (type) safe to me - I would use `uint8_t` to make it explicit.

In my opinion, we should focus on fixing the bug first, than we can discuss API changes for type safeness. The bug is fixable without an API change, so we should do that first, then do an API change if necessary. This not only makes the merge of this bug fix faster, but also allows for a better understanding of the thought process of the API change when looking at the code's history.

-- 
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/11624#discussion_r290651142
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.riot-os.org/pipermail/notifications/attachments/20190605/0910c6a8/attachment.html>


More information about the notifications mailing list