gnrc_pktbuf_static: fix #5748 #6086
Conversation
miri64
added
Type: bug
|
With |
|
Mh okay. Then let's postpone this one for now. I will investigate this for 2017.01. |
| @@ -437,6 +437,7 @@ static void *_pktbuf_alloc(size_t size) | |||
| DEBUG("pktbuf: no space left in packet buffer\n"); | |||
| return NULL; | |||
| } | |||
| /* _unused_t struct would fit => add new space at ptr */ | |||
| if (sizeof(_unused_t) > (ptr->size - size)) { | |||
There was a problem hiding this comment.
shouldn't this be if ((_unused_t != NULL) && (sizeof(_unused_t) > (ptr->size - size))), compare L454 where _first_unused = NULL;?
| @@ -447,7 +448,12 @@ static void *_pktbuf_alloc(size_t size) | |||
| } | |||
| else { | |||
| _unused_t *new = (_unused_t *)(((uint8_t *)ptr) + size); | |||
| if (prev == NULL) { /* ptr was _first_unused */ | |||
|
|
|||
| if (((((uint8_t *)new) - &_pktbuf[0]) + sizeof(_unused_t)) > GNRC_PKTBUF_SIZE) { | |||
There was a problem hiding this comment.
A lot of parens - but still not enough. ;-) Should be &(_pktbuf[0])).
There was a problem hiding this comment.
To me: since ever. ;-) Just to make clear that the address of _pktbuf[0] is meant and not the address of _pktbuf.
There was a problem hiding this comment.
IMHO that's perfectly clear, because even forgetting all about operator priorities it still wouldn't make sense to me to give & priority over [], but fine.
|
IIRC the last time we talked about this PR you wanted to look into it again or should we proceed and merge this ASAP? |
I would rather merge it ASAP. The error with the crash on large networks seems to be unrelated. |
miri64
force-pushed
the
gnrc_pktbuf_static/fix/i5748
branch
from
d46c603 to
5e236cd
Compare
|
Squashed |
I think I found the problem causing #5748 (and maybe even #4048): If the packet buffer gets full it might happen that the
_unused_tsegment is written to the very end, though it will overflow. This PR fixes that.