[GitHub] mkiiskila commented on a change in pull request #715: os_mempool; block size for memory pool has to allow storage for free ?

[GitBox <gi...@apache.org>]

mkiiskila commented on a change in pull request #715: os_mempool; block size for memory pool has to allow storage for free ?
URL: https://github.com/apache/mynewt-core/pull/715#discussion_r159247726
 
 

 ##########
 File path: kernel/os/src/os_mempool.c
 ##########
 @@ -90,7 +90,7 @@ os_mempool_init(struct os_mempool *mp, int blocks, int block_size,
     struct os_memblock *block_ptr;
 
     /* Check for valid parameters */
-    if (!mp || (blocks < 0) || (block_size <= 0)) {
+    if (!mp || (blocks < 0) || (block_size < sizeof(struct os_memblock))) {
 
 Review comment:
   Hi! Issue would appear if the user did not use OS_MEMPOOL_BYTES(), and specified block_size of, say 2 bytes. Clearly the test against > 0 is not correct, and we should have at least a pointer's worth of memory on every block.
   As you have seen from the code, we typecast the beginning of a block into struct os_memblock and link it through that struct.
   
   I'd be ok with removing the argument checks altogether, in which case the responsibility of passing correct arguments is up to the user. But if we keep the checks, then they should be meaningful.
   
   Hope this helps,
   M

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services