From fceabd82fc8c7d99f82f60471d3167062f3c1b92 Mon Sep 17 00:00:00 2001 From: Damian Krolik Date: Thu, 5 Sep 2024 16:32:09 +0200 Subject: [PATCH] nrf_rpc: enable zcbor stop_on_error flag for decoding MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- subsys/net/openthread/rpc/common/ot_rpc_common.c | 2 +- subsys/nrf_rpc/nrf_rpc_serialize.c | 9 ++++++++- tests/subsys/net/openthread/rpc/server/prj.conf | 2 +- tests/subsys/net/openthread/rpc/server/src/cmsng_suite.c | 7 +++++-- tests/subsys/nfc/rpc/server/prj.conf | 3 +-- tests/subsys/nfc/rpc/server/src/t2t_suite.c | 7 +++++-- tests/subsys/nfc/rpc/server/src/t4t_suite.c | 4 +++- west.yml | 2 +- 8 files changed, 25 insertions(+), 11 deletions(-) diff --git a/subsys/net/openthread/rpc/common/ot_rpc_common.c b/subsys/net/openthread/rpc/common/ot_rpc_common.c index a0f46e4164e6..5c62ee447e81 100644 --- 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 index b74cc7d26971..beeac98d0b0a 100644 --- 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 index 507d5eb995f9..50b4d02d1ab3 100644 --- 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 index beefa738bff3..caf930c910f4 100644 --- a/tests/subsys/net/openthread/rpc/server/src/cmsng_suite.c +++ b/tests/subsys/net/openthread/rpc/server/src/cmsng_suite.c @@ -27,6 +27,7 @@ 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); \ @@ -34,7 +35,8 @@ FAKE_VALUE_FUNC(otError, otDatasetGetActive, otInstance *, otOperationalDataset 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 index 9a7612ec9832..c61eb856a38f 100644 --- 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 index eff988cd48dc..786bd13e97bb 100644 --- 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,6 +65,7 @@ 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); @@ -70,7 +73,7 @@ ZTEST(nfc_rpc_t2t_srv, test_nfc_t2t_setup) 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 index 39775a944755..18bd5a33eed4 100644 --- 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,6 +63,7 @@ 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); @@ -69,7 +71,7 @@ ZTEST(nfc_rpc_t4t_srv, test_nfc_t4t_setup) 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 index fa75bc8bc5f5..5b90c59dc492 100644 --- 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