[GitHub] [mynewt-nimble] RoshanESP commented on a diff in pull request #1557: nimble/host : Added leading_space as parameter to os_msys_get_pkthdr

["RoshanESP (via GitHub)" <gi...@apache.org>]

RoshanESP commented on code in PR #1557:
URL: https://github.com/apache/mynewt-nimble/pull/1557#discussion_r1283998557


##########
nimble/host/src/ble_hs_mbuf.c:
##########
@@ -31,9 +31,9 @@ ble_hs_mbuf_gen_pkt(uint16_t leading_space)
     int rc;
 
 #if MYNEWT_VAL(BLE_CONTROLLER)
-    om = os_msys_get_pkthdr(0, sizeof(struct ble_mbuf_hdr));
+    om = os_msys_get_pkthdr(leading_space, sizeof(struct ble_mbuf_hdr));
 #else
-    om = os_msys_get_pkthdr(0, 0);
+    om = os_msys_get_pkthdr(leading_space, 0);

Review Comment:
   Hi @sjanc , 
   Actually, the dsize param in os_msys_get_pkthdr() is used only to find the pool and not to allocate the packet header. 
   https://github.com/apache/mynewt-nimble/blob/e718a2fb22a56141ddb6a63cab875489a6a08f25/porting/nimble/src/os_mbuf.c#L192-L209
   In os_msys_get_pktdhr() dsize is used only to find the pool, by passing the appropriate dsize, the possibility of smaller mbuf getting allocated can be eliminated. 
   `while dsize param in os_msys_get_pkthdr() is useful only of one know exact data size that will be used`. 
   I think passing dsize doesn't allocate the mbuf of exact size, rather it allocates the mbuf that has data_size >= dsize. 
   
   As the ble_hs_mbuf_gen_pkt is static function and called only with leading_space required for protocol headers(which is small enough), it is not causing any problem. 
   If someone keeps the msys block_size small enough then this may cause error as the check 
   `if (om->om_omp->omp_databuf_len < leading_space) ` will fail. So, passing the leading_space to os_msys_get_pkthdr is safer. 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@mynewt.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org