You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mynewt.apache.org by cc...@apache.org on 2016/12/02 19:28:23 UTC
incubator-mynewt-core git commit: MYNEWT-324 BLE Host - Don't use req
buf for wr.rsp
Repository: incubator-mynewt-core
Updated Branches:
refs/heads/develop e679552c1 -> 9f9377088
MYNEWT-324 BLE Host - Don't use req buf for wr.rsp
Never reuse the request mbuf for a write response. The rationale is
explained in the comment below:
/**
* Notes on buffer reuse:
* Most request handlers reuse the request buffer for the reponse. This
* is done to prevent out-of-memory conditions. However, there are two
* handlers which do not reuse the request buffer:
* 1. Write request.
* 2. Indicate request.
*
* Both of these handlers attempt to allocate a new buffer for the
* response prior to processing the request. If allocation fails, the
* request is not processed, and the request buffer is reused for the
* transmission of an "insufficient resources" ATT error response.
* These handlers don't reuse the request mbuf for an affirmative
* response because the buffer contains the attribute data that gets
* passed to the application callback. The application may choose to
* retain the mbuf during the callback, so the stack
*/
Project: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/repo
Commit: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/commit/9f937708
Tree: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/tree/9f937708
Diff: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/diff/9f937708
Branch: refs/heads/develop
Commit: 9f9377088815bd987564064c433d43552ad3e1ef
Parents: e679552
Author: Christopher Collins <cc...@apache.org>
Authored: Fri Dec 2 11:23:58 2016 -0800
Committer: Christopher Collins <cc...@apache.org>
Committed: Fri Dec 2 11:23:58 2016 -0800
----------------------------------------------------------------------
net/nimble/host/src/ble_att_svr.c | 62 +++++++++++++++---------
net/nimble/host/test/src/ble_att_svr_test.c | 9 ++--
2 files changed, 45 insertions(+), 26 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/blob/9f937708/net/nimble/host/src/ble_att_svr.c
----------------------------------------------------------------------
diff --git a/net/nimble/host/src/ble_att_svr.c b/net/nimble/host/src/ble_att_svr.c
index 3e5f277..45299de 100644
--- a/net/nimble/host/src/ble_att_svr.c
+++ b/net/nimble/host/src/ble_att_svr.c
@@ -25,6 +25,25 @@
#include "host/ble_uuid.h"
#include "ble_hs_priv.h"
+/**
+ * ATT server - Attribute Protocol
+ *
+ * Notes on buffer reuse:
+ * Most request handlers reuse the request buffer for the reponse. This is
+ * done to prevent out-of-memory conditions. However, there are two handlers
+ * which do not reuse the request buffer:
+ * 1. Write request.
+ * 2. Indicate request.
+ *
+ * Both of these handlers attempt to allocate a new buffer for the response
+ * prior to processing the request. If allocation fails, the request is not
+ * processed, and the request buffer is reused for the transmission of an
+ * "insufficient resources" ATT error response. These handlers don't reuse the
+ * request mbuf for an affirmative response because the buffer contains the
+ * attribute data that gets passed to the application callback. The
+ * application may choose to retain the mbuf during the callback, so the stack
+ */
+
STAILQ_HEAD(ble_att_svr_entry_list, ble_att_svr_entry);
static struct ble_att_svr_entry_list ble_att_svr_list;
@@ -2031,18 +2050,12 @@ ble_att_svr_build_write_rsp(struct os_mbuf **rxom, struct os_mbuf **out_txom,
uint8_t *dst;
int rc;
- /* Just reuse the request buffer for the response if the application didn't
- * retain it.
+ /* Allocate a new buffer for the response. A write response never reuses
+ * the request buffer. See the note at the top of this file for details.
*/
- if (*rxom != NULL) {
- txom = *rxom;
- *rxom = NULL;
- os_mbuf_adj(txom, OS_MBUF_PKTLEN(txom));
- } else {
- rc = ble_att_svr_pkt(rxom, &txom, att_err);
- if (rc != 0) {
- goto done;
- }
+ rc = ble_att_svr_pkt(rxom, &txom, att_err);
+ if (rc != 0) {
+ goto done;
}
dst = os_mbuf_extend(txom, BLE_ATT_WRITE_RSP_SZ);
@@ -2090,21 +2103,25 @@ ble_att_svr_rx_write(uint16_t conn_handle, struct os_mbuf **rxom)
BLE_ATT_LOG_CMD(0, "write req", conn_handle,
ble_att_write_cmd_log, &req);
+ err_handle = req.bawq_handle;
+
+ /* Allocate the write response. This must be done prior to processing the
+ * request. See the note at the top of this file for details.
+ */
+ rc = ble_att_svr_build_write_rsp(rxom, &txom, &att_err);
+ if (rc != 0) {
+ goto done;
+ }
+
/* Strip the request base from the front of the mbuf. */
os_mbuf_adj(*rxom, BLE_ATT_WRITE_REQ_BASE_SZ);
rc = ble_att_svr_write_handle(conn_handle, req.bawq_handle, 0, rxom,
&att_err);
if (rc != 0) {
- err_handle = req.bawq_handle;
goto done;
}
- rc = ble_att_svr_build_write_rsp(rxom, &txom, &att_err);
- if (rc != 0) {
- err_handle = req.bawq_handle;
- goto done;
- }
BLE_ATT_LOG_EMPTY_CMD(1, "write rsp", conn_handle);
rc = 0;
@@ -2651,6 +2668,10 @@ ble_att_svr_build_indicate_rsp(struct os_mbuf **rxom,
uint8_t *dst;
int rc;
+ /* Allocate a new buffer for the response. An indicate response never
+ * reuses the request buffer. See the note at the top of this file for
+ * details.
+ */
rc = ble_att_svr_pkt(rxom, &txom, out_att_err);
if (rc != 0) {
goto done;
@@ -2710,11 +2731,8 @@ ble_att_svr_rx_indicate(uint16_t conn_handle, struct os_mbuf **rxom)
goto done;
}
- /* Ensure we can allocate a response before processing the indication.
- * We can't reuse the request mbuf because it contains the data that needs
- * to be passed to the application callback. The application may choose to
- * retain the mbuf during the callback, so we can't just reuse the mbuf
- * after executing the callback either.
+ /* Allocate the indicate response. This must be done prior to processing
+ * the request. See the note at the top of this file for details.
*/
rc = ble_att_svr_build_indicate_rsp(rxom, &txom, &att_err);
if (rc != 0) {
http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core/blob/9f937708/net/nimble/host/test/src/ble_att_svr_test.c
----------------------------------------------------------------------
diff --git a/net/nimble/host/test/src/ble_att_svr_test.c b/net/nimble/host/test/src/ble_att_svr_test.c
index 4a1ea61..d794c49 100644
--- a/net/nimble/host/test/src/ble_att_svr_test.c
+++ b/net/nimble/host/test/src/ble_att_svr_test.c
@@ -1969,17 +1969,18 @@ TEST_CASE(ble_att_svr_test_oom)
ble_hs_test_util_verify_tx_err_rsp(BLE_ATT_OP_READ_GROUP_TYPE_REQ, 11,
BLE_ATT_ERR_ATTR_NOT_FOUND);
- /*** Write; always respond affirmatively, even when no mbufs. */
+ /*** Write. */
ble_hs_test_util_prev_tx_dequeue();
/* Receive a request. */
rc = ble_hs_test_util_rx_att_write_req(conn_handle, 1,
((uint8_t[1]){1}), 1);
- TEST_ASSERT_FATAL(rc == 0);
+ TEST_ASSERT_FATAL(rc == BLE_HS_ENOMEM);
- /* Ensure we were able to send a real response. */
+ /* Ensure we were able to send an error response. */
ble_hs_test_util_tx_all();
- ble_hs_test_util_verify_tx_write_rsp();
+ ble_hs_test_util_verify_tx_err_rsp(BLE_ATT_OP_WRITE_REQ, 1,
+ BLE_ATT_ERR_INSUFFICIENT_RES);
/*** Write command; no response. */
ble_hs_test_util_prev_tx_dequeue();