[GitHub] [mynewt-nimble] ccollins476ad commented on a change in pull request #821: nimble/hs: Fix flow control

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

ccollins476ad commented on a change in pull request #821:
URL: https://github.com/apache/mynewt-nimble/pull/821#discussion_r430039571



##########
File path: nimble/include/nimble/ble.h
##########
@@ -177,8 +177,8 @@ struct ble_mbuf_hdr
 #define BLE_MBUF_MEMBLOCK_OVERHEAD      \
     (sizeof(struct os_mbuf) + BLE_MBUF_PKTHDR_OVERHEAD)
 
-/* Length of host user header.  Only contains the peer's connection handle. */
-#define BLE_MBUF_HS_HDR_LEN     (2)
+#define BLE_MBUF_HS_HDR_LEN     (0)

Review comment:
       This comment is definitely in "nitpick territory" so feel free to ignore it.  IMO, this constant reduces clarity without any benefit.  It would be more consistent and easily understood to just hardcode 0 at the allocation site.

##########
File path: nimble/host/src/ble_hs_flow.c
##########
@@ -40,6 +40,23 @@ static ble_npl_event_fn ble_hs_flow_event_cb;
 
 static struct ble_npl_event ble_hs_flow_ev;
 
+/* Connection handle associated with each mbuf in ACL pool */
+static uint16_t ble_hs_flow_mbuf_conn_handle[ MYNEWT_VAL(BLE_ACL_BUF_COUNT) ];
+
+static inline int
+ble_hs_flow_mbuf_index(const struct os_mbuf *om)
+{
+    const struct os_mempool *mp = om->om_omp->omp_pool;
+    uint32_t addr = (uint32_t)om;

Review comment:
       I would use `uintptr_t` here.




----------------------------------------------------------------
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.

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