nrf_rpc: enable zcbor stop_on_error flag for decoding

1. The zcbor's stop_on_error flag, which defaults to false, makes the decoder stop decoding any subsequent items after a decoding error occurs, meaning that all subsequent decoding attempts fail early until the error is cleared with zcbor_pop_error() function. This flag is already set for the encoder yet it was omitted for the decoder although some nRF RPC decoder functions seem to rely the flag being set as they only check the decoding result after trying to decode all the command or response arguments. 2. Fix a function for checking if the next data item is null. The function would fail the decoding if the next item was another CBOR simple type, such as a bool constant. Signed-off-by: Damian Krolik <[email protected]>
nrfconnect · Sep 9, 2024 · fceabd8 · fceabd8
1 parent ba85bce
commit fceabd8
Show file tree

Hide file tree

Showing 8 changed files with 25 additions and 11 deletions.
diff --git a/subsys/net/openthread/rpc/common/ot_rpc_common.c b/subsys/net/openthread/rpc/common/ot_rpc_common.c
@@ -103,7 +103,7 @@ void ot_rpc_decode_dataset(const struct nrf_rpc_group *group, struct nrf_rpc_cbo
 {
 	otOperationalDataset *dataset = *(otOperationalDataset **)handler_data;
 
-	if (zcbor_nil_expect(ctx->zs, NULL)) {
+	if (nrf_rpc_decode_is_null(ctx)) {
 		*(otOperationalDataset **)handler_data = NULL;
 		return;
 	}

diff --git a/subsys/nrf_rpc/nrf_rpc_serialize.c b/subsys/nrf_rpc/nrf_rpc_serialize.c
@@ -139,8 +139,15 @@ bool nrf_rpc_decode_is_null(struct nrf_rpc_cbor_ctx *ctx)
 		return true;
 	}
 
-	if (ctx->zs->constant_state->error == ZCBOR_ERR_WRONG_TYPE) {
+	switch (ctx->zs->constant_state->error) {
+	/* The next data item is not a simple value (Major 7) */
+	case ZCBOR_ERR_WRONG_TYPE:
+	/* The next data item is a simple value other than null (for example, a bool) */
+	case ZCBOR_ERR_WRONG_VALUE:
 		zcbor_pop_error(ctx->zs);
+		break;
+	default:
+		break;
 	}
 
 	return false;

diff --git a/tests/subsys/net/openthread/rpc/server/prj.conf b/tests/subsys/net/openthread/rpc/server/prj.conf
@@ -10,7 +10,7 @@ CONFIG_TEST_RANDOM_GENERATOR=y
 
 CONFIG_OPENTHREAD_RPC=y
 CONFIG_OPENTHREAD_RPC_SERVER=y
-CONFIG_NRF_RPC_CBKPROXY_OUT_SLOTS=0
+CONFIG_NRF_RPC_CALLBACK_PROXY=n
 
 CONFIG_MOCK_NRF_RPC=y
 CONFIG_MOCK_NRF_RPC_TRANSPORT=y

diff --git a/tests/subsys/net/openthread/rpc/server/src/cmsng_suite.c b/tests/subsys/net/openthread/rpc/server/src/cmsng_suite.c
@@ -27,14 +27,16 @@ FAKE_VALUE_FUNC(otError, otDatasetSetActiveTlvs, otInstance *, const otOperation
 FAKE_VALUE_FUNC(otError, otDatasetGetActiveTlvs, otInstance *, otOperationalDatasetTlvs *);
 FAKE_VALUE_FUNC(otError, otDatasetSetActive, otInstance *, const otOperationalDataset *);
 FAKE_VALUE_FUNC(otError, otDatasetGetActive, otInstance *, otOperationalDataset *);
+FAKE_VALUE_FUNC(void *, nrf_rpc_cbkproxy_out_get, int, void *);
 
 #define FOREACH_FAKE(f)                                                                            \
 	f(otThreadDiscover);                                                                       \
 	f(otDatasetIsCommissioned);                                                                \
 	f(otDatasetSetActiveTlvs);                                                                 \
 	f(otDatasetGetActiveTlvs);                                                                 \
 	f(otDatasetSetActive);                                                                     \
-	f(otDatasetGetActive)
+	f(otDatasetGetActive);                                                                     \
+	f(nrf_rpc_cbkproxy_out_get);
 
 extern uint64_t ot_thread_discover_cb_encoder(uint32_t callback_slot, uint32_t _rsv0,
 					      uint32_t _rsv1, uint32_t _ret,
@@ -61,6 +63,7 @@ static void tc_setup(void *f)
  */
 ZTEST(ot_rpc_commissioning_server, test_otThreadDiscover)
 {
+	nrf_rpc_cbkproxy_out_get_fake.return_val = (void *)0xfacecafe;
 	otThreadDiscover_fake.return_val = OT_ERROR_NONE;
 
 	mock_nrf_rpc_tr_expect_add(RPC_RSP(OT_ERROR_NONE), NO_RSP);
@@ -74,7 +77,7 @@ ZTEST(ot_rpc_commissioning_server, test_otThreadDiscover)
 	zassert_equal(otThreadDiscover_fake.arg2_val, 0x4321);
 	zassert_equal(otThreadDiscover_fake.arg3_val, false);
 	zassert_equal(otThreadDiscover_fake.arg4_val, true);
-	zassert_equal_ptr(otThreadDiscover_fake.arg5_val, NULL);
+	zassert_equal_ptr(otThreadDiscover_fake.arg5_val, (void *)0xfacecafe);
 	zassert_equal_ptr(otThreadDiscover_fake.arg6_val, (void *)0xcafeface);
 }
 

diff --git a/tests/subsys/nfc/rpc/server/prj.conf b/tests/subsys/nfc/rpc/server/prj.conf
@@ -8,8 +8,7 @@ CONFIG_ZTEST=y
 
 CONFIG_NFC_RPC=y
 CONFIG_NFC_RPC_SERVER=y
-
-CONFIG_NRF_RPC_CBKPROXY_OUT_SLOTS=0
+CONFIG_NRF_RPC_CALLBACK_PROXY=n
 
 CONFIG_MOCK_NRF_RPC=y
 CONFIG_MOCK_NRF_RPC_TRANSPORT=y

diff --git a/tests/subsys/nfc/rpc/server/src/t2t_suite.c b/tests/subsys/nfc/rpc/server/src/t2t_suite.c
@@ -29,6 +29,7 @@ FAKE_VALUE_FUNC(int, nfc_t2t_internal_set, const uint8_t *, size_t);
 FAKE_VALUE_FUNC(int, nfc_t2t_emulation_start);
 FAKE_VALUE_FUNC(int, nfc_t2t_emulation_stop);
 FAKE_VALUE_FUNC(int, nfc_t2t_done);
+FAKE_VALUE_FUNC(void *, nrf_rpc_cbkproxy_out_get, int, void *);
 
 #define FOREACH_FAKE(f)                                                                            \
 	f(nfc_t2t_setup);                                                                          \
@@ -39,7 +40,8 @@ FAKE_VALUE_FUNC(int, nfc_t2t_done);
 	f(nfc_t2t_internal_set);                                                                   \
 	f(nfc_t2t_emulation_start);                                                                \
 	f(nfc_t2t_emulation_stop);                                                                 \
-	f(nfc_t2t_done);
+	f(nfc_t2t_done);                                                                           \
+	f(nrf_rpc_cbkproxy_out_get);
 
 extern uint64_t nfc_t2t_cb_encoder(uint32_t callback_slot, uint32_t _rsv0, uint32_t _rsv1,
 				   uint32_t _ret, void *context, nfc_t2t_event_t event,
@@ -63,14 +65,15 @@ static void tc_setup(void *f)
 /* Test reception of nfc_t2t_setup command. */
 ZTEST(nfc_rpc_t2t_srv, test_nfc_t2t_setup)
 {
+	nrf_rpc_cbkproxy_out_get_fake.return_val = (void *)0xfacecafe;
 	nfc_t2t_setup_fake.return_val = 0;
 
 	mock_nrf_rpc_tr_expect_add(RPC_RSP(0), NO_RSP);
 	mock_nrf_rpc_tr_receive(RPC_CMD(NFC_RPC_CMD_T2T_SETUP, 0, CBOR_UINT32(0xcafeface)));
 	mock_nrf_rpc_tr_expect_done();
 
 	zassert_equal(nfc_t2t_setup_fake.call_count, 1);
-	zassert_equal_ptr(nfc_t2t_setup_fake.arg0_val, NULL);
+	zassert_equal_ptr(nfc_t2t_setup_fake.arg0_val, (void *)0xfacecafe);
 	zassert_equal_ptr(nfc_t2t_setup_fake.arg1_val, (void *)0xcafeface);
 }
 

diff --git a/tests/subsys/nfc/rpc/server/src/t4t_suite.c b/tests/subsys/nfc/rpc/server/src/t4t_suite.c
@@ -28,6 +28,7 @@ FAKE_VALUE_FUNC(int, nfc_t4t_parameter_get, nfc_t4t_param_id_t, void *, size_t *
 FAKE_VALUE_FUNC(int, nfc_t4t_emulation_start);
 FAKE_VALUE_FUNC(int, nfc_t4t_emulation_stop);
 FAKE_VALUE_FUNC(int, nfc_t4t_done);
+DECLARE_FAKE_VALUE_FUNC(void *, nrf_rpc_cbkproxy_out_get, int, void *);
 
 #define FOREACH_FAKE(f)                                                                            \
 	f(nfc_t4t_setup);                                                                          \
@@ -62,14 +63,15 @@ static void tc_setup(void *f)
 /* Test reception of nfc_t4t_setup command. */
 ZTEST(nfc_rpc_t4t_srv, test_nfc_t4t_setup)
 {
+	nrf_rpc_cbkproxy_out_get_fake.return_val = (void *)0xfacecafe;
 	nfc_t4t_setup_fake.return_val = 0;
 
 	mock_nrf_rpc_tr_expect_add(RPC_RSP(0), NO_RSP);
 	mock_nrf_rpc_tr_receive(RPC_CMD(NFC_RPC_CMD_T4T_SETUP, 0, CBOR_UINT32(0xcafeface)));
 	mock_nrf_rpc_tr_expect_done();
 
 	zassert_equal(nfc_t4t_setup_fake.call_count, 1);
-	zassert_equal_ptr(nfc_t4t_setup_fake.arg0_val, NULL);
+	zassert_equal_ptr(nfc_t4t_setup_fake.arg0_val, (void *)0xfacecafe);
 	zassert_equal_ptr(nfc_t4t_setup_fake.arg1_val, (void *)0xcafeface);
 }
 

diff --git a/west.yml b/west.yml
@@ -157,7 +157,7 @@ manifest:
     - name: nrfxlib
       repo-path: sdk-nrfxlib
       path: nrfxlib
-      revision: a21499e8ac4bbf00e41879ade74dc95a26f7edb9
+      revision: pull/1461/head
     - name: trusted-firmware-m
       repo-path: sdk-trusted-firmware-m
       path: modules/tee/tf-m/trusted-firmware-m