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

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;
 	}

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;

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

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);
 }
 

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

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);
 }
 

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);
 }
 

west.yml

@@ -157,7 +157,7 @@ manifest:
     - name: nrfxlib
       repo-path: sdk-nrfxlib
       path: nrfxlib
-      revision: 82c2271901d054da32217c5a9622c7ab66639b41
+      revision: pull/1461/head
     - name: trusted-firmware-m
       repo-path: sdk-trusted-firmware-m
       path: modules/tee/tf-m/trusted-firmware-m