2015-11-11 09:58 AM
Hi,
This code generated by STM32Cube with FreeRTOS looks like it has a bug. To reproduce, create a small pool. Allocate a single item that is not free'd then, allocate and deallocate sucessive items. The pool will stop allocating, even though there are free slots.The code at fault is, index = pool_id->currentIndex + i; if (index >= pool_id->pool_sz) { index = 0; }currentIndex is never reset, and so for a queue size of say 8, if currentIndex = 7, then for all i (1,2,3..) in the loop, currentindex + i will be greater than pool_sz. and index will be set to 0, so only markers[0] gets checked continiously. If index 0 is not free, then the pool will not allocate, and will break in this conditionThe above section should be something like index = (pool_id->currentIndex + i) % pool_id->pool_sz;FULL FUNCTION:void *osPoolAlloc (osPoolId pool_id){ int dummy = 0; void *p = NULL; uint32_t i; uint32_t index; if (inHandlerMode()) { dummy = portSET_INTERRUPT_MASK_FROM_ISR(); } else { vPortEnterCritical(); } for (i = 0; i < pool_id->pool_sz; i++) { index = pool_id->currentIndex + i; if (index >= pool_id->pool_sz) { index = 0; } if (pool_id->markers[index] == 0) { pool_id->markers[index] = 1; p = (void *)((uint32_t)(pool_id->pool) + (index * pool_id->item_sz)); pool_id->currentIndex = index; break; } } if (inHandlerMode()) { portCLEAR_INTERRUPT_MASK_FROM_ISR(dummy); } else { vPortExitCritical(); } return p;}2015-11-12 01:44 AM
Hello,
maybe there is another error in function
osPoolCAlloc
void
*
osPoolCAlloc
(
osPoolId
pool_id)
{
void
*p = osPoolAlloc(pool_id);
if
(p != NULL)
{
//
memset
(p, 0,
sizeof
(pool_id->pool_sz));
ERROR this is size of uint32_t = 4
memset
(p, 0, pool_id->
item_sz
); //
should be
}
return
p;
}
my proposel for
osPoolAlloc()
void
*
osPoolAlloc
(
osPoolId
pool_id)
{
int
dummy = 0;
void
*p = NULL;
uint32_t
i;
uint32_t
index;
if
(inHandlerMode()) {
dummy = portSET_INTERRUPT_MASK_FROM_ISR();
}
else
{
vPortEnterCritical();
}
index=pool_id->
currentIndex
;
// +++ new
for
(i = 0; i < pool_id->
pool_sz
; i++) {
// index = pool_id->currentIndex + i; // --- old
if
(index >= pool_id->
pool_sz
) {
index = 0;
}
if
(pool_id->
markers
[index] == 0) {
pool_id->
markers
[index] = 1;
p = (
void
*)((
uint32_t
)(pool_id->
pool
) + (index * pool_id->
item_sz
));
pool_id->
currentIndex
= index;
break
;
}
index++;
// +++ new
}
if
(inHandlerMode()) {
portCLEAR_INTERRUPT_MASK_FROM_ISR(dummy);
}
else
{
vPortExitCritical();
}
return
p;
}
maybe in
osPoolFree
currentIndex
could be set to the last freed block, so
alloc does not need to search for a free block.
osStatus
osPoolFree
(
osPoolId
pool_id,
void
*block)
{
uint32_t
index;
if
(pool_id == NULL) {
return
osErrorParameter
;
}
if
(block == NULL) {
return
osErrorParameter
;
}
if
(block < pool_id->
pool
) {
return
osErrorParameter
;
}
index = (
uint32_t
)block - (
uint32_t
)(pool_id->
pool
);
if
(index % pool_id->
item_sz
) {
return
osErrorParameter
;
}
index = index / pool_id->
item_sz
;
if
(index >= pool_id->
pool_sz
) {
return
osErrorParameter
;
}
pool_id->
markers
[index] = 0;
pool_id->currentIndex=index; // +++ next
alloc
will get this free block
return
osOK
;
}
A better solution would be to use a linked list with free memory packets.
This would avoid searching for a free packet on allocation.
best regards