From 223aeda2825e7ef09f9e5b90e98872e36a9ad3da Mon Sep 17 00:00:00 2001 From: Neels Hofmeyr Date: Sun, 10 Apr 2022 00:28:07 +0200 Subject: [PATCH] ranap_rab_ass_req_encode(): return msgb ranap_rab_ass_req_encode() forms a msgb, then copies the data to a buffer provided by the caller. Instead, just return the msgb. This removes one unnecessary memcpy() and simplifies some code. In ranap_rab_ass_test.c, actually ensure the correct size of the returned data. See also the fix of expected test data in patch Ifb98a52e56db1227a834c0d7b7a260314d9f547e Related: SYS#5895 Change-Id: I85e715326e1d8f4f301f82f78da109f1a7a92f30 --- include/osmocom/hnbgw/ranap_rab_ass.h | 3 +-- src/osmo-hnbgw/mgw_fsm.c | 9 ++++----- src/osmo-hnbgw/ranap_rab_ass.c | 16 +++------------- tests/ranap_rab_ass/Makefile.am | 4 ++++ tests/ranap_rab_ass/ranap_rab_ass_test.c | 20 ++++++++++++-------- tests/ranap_rab_ass/ranap_rab_ass_test.ok | 2 +- 6 files changed, 25 insertions(+), 29 deletions(-) diff --git a/include/osmocom/hnbgw/ranap_rab_ass.h b/include/osmocom/hnbgw/ranap_rab_ass.h index 13fd2df..ae288af 100644 --- a/include/osmocom/hnbgw/ranap_rab_ass.h +++ b/include/osmocom/hnbgw/ranap_rab_ass.h @@ -1,7 +1,6 @@ #pragma once -int ranap_rab_ass_req_encode(uint8_t *data, unsigned int len, - RANAP_RAB_AssignmentRequestIEs_t *rab_assignment_request_ies); +struct msgb *ranap_rab_ass_req_encode(RANAP_RAB_AssignmentRequestIEs_t *rab_assignment_request_ies); int ranap_rab_ass_resp_encode(uint8_t *data, unsigned int len, RANAP_RAB_AssignmentResponseIEs_t *rab_assignment_response_ies); diff --git a/src/osmo-hnbgw/mgw_fsm.c b/src/osmo-hnbgw/mgw_fsm.c index a343e80..8e42f58 100644 --- a/src/osmo-hnbgw/mgw_fsm.c +++ b/src/osmo-hnbgw/mgw_fsm.c @@ -235,20 +235,19 @@ static void mgw_fsm_assign_onenter(struct osmo_fsm_inst *fi, uint32_t prev_state { struct mgw_fsm_priv *mgw_fsm_priv = fi->priv; struct hnbgw_context_map *map = mgw_fsm_priv->map; - uint8_t encoded[IUH_MSGB_SIZE]; RANAP_RAB_AssignmentRequestIEs_t *ies; - int rc; + struct msgb *msg; ies = &mgw_fsm_priv->ranap_rab_ass_req_message->msg.raB_AssignmentRequestIEs; - rc = ranap_rab_ass_req_encode(encoded, sizeof(encoded), ies); - if (rc < 0) { + msg = ranap_rab_ass_req_encode(ies); + if (!msg) { LOGPFSML(fi, LOGL_ERROR, "failed to re-encode RAB-AssignmentRequest message\n"); osmo_fsm_inst_state_chg(fi, MGW_ST_FAILURE, 0, 0); return; } LOGPFSML(fi, LOGL_DEBUG, "forwarding modified RAB-AssignmentRequest to HNB\n"); - rua_tx_dt(map->hnb_ctx, map->is_ps, map->rua_ctx_id, encoded, rc); + rua_tx_dt(map->hnb_ctx, map->is_ps, map->rua_ctx_id, msg->data, msg->len); } static void mgw_fsm_assign(struct osmo_fsm_inst *fi, uint32_t event, void *data) diff --git a/src/osmo-hnbgw/ranap_rab_ass.c b/src/osmo-hnbgw/ranap_rab_ass.c index 5930451..78805b8 100644 --- a/src/osmo-hnbgw/ranap_rab_ass.c +++ b/src/osmo-hnbgw/ranap_rab_ass.c @@ -37,20 +37,18 @@ * \ptmap[in] len length of user provided memory to store resulting ASN.1 encoded message. * \ptmap[in] ies user provided memory with RANAP_RAB_AssignmentRequestIEs. * \returns resulting message length on success; negative on error. */ -int ranap_rab_ass_req_encode(uint8_t *data, unsigned int len, - RANAP_RAB_AssignmentRequestIEs_t *rab_assignment_request_ies) +struct msgb *ranap_rab_ass_req_encode(RANAP_RAB_AssignmentRequestIEs_t *rab_assignment_request_ies) { int rc; struct msgb *msg; RANAP_RAB_AssignmentRequest_t _rab_assignment_request; RANAP_RAB_AssignmentRequest_t *rab_assignment_request = &_rab_assignment_request; - memset(data, 0, len); memset(rab_assignment_request, 0, sizeof(*rab_assignment_request)); rc = ranap_encode_rab_assignmentrequesties(rab_assignment_request, rab_assignment_request_ies); if (rc < 0) - return -EINVAL; + return NULL; /* generate an Initiating Mesasage */ msg = ranap_generate_initiating_message(RANAP_ProcedureCode_id_RAB_Assignment, @@ -60,15 +58,7 @@ int ranap_rab_ass_req_encode(uint8_t *data, unsigned int len, /* 'msg' has been generated, we cann now release the input 'out' */ ASN_STRUCT_FREE_CONTENTS_ONLY(asn_DEF_RANAP_RAB_AssignmentRequest, rab_assignment_request); - if (!msg) - return -EINVAL; - if (msg->len > len) - return -EINVAL; - - memcpy(data, msg->data, msg->len); - rc = msg->len; - msgb_free(msg); - return rc; + return msg; } /*! Encode RABAP RAB AssignmentRequest from RANAP_RAB_AssignmentResponseIEs. diff --git a/tests/ranap_rab_ass/Makefile.am b/tests/ranap_rab_ass/Makefile.am index 8d1df7a..70ca6f6 100644 --- a/tests/ranap_rab_ass/Makefile.am +++ b/tests/ranap_rab_ass/Makefile.am @@ -35,3 +35,7 @@ ranap_rab_ass_test_LDADD = \ $(COVERAGE_LDFLAGS) \ $(top_builddir)/src/osmo-hnbgw/ranap_rab_ass.o \ $(NULL) + +.PHONY: update_exp +update_exp: + $(builddir)/ranap_rab_ass_test >$(srcdir)/ranap_rab_ass_test.ok diff --git a/tests/ranap_rab_ass/ranap_rab_ass_test.c b/tests/ranap_rab_ass/ranap_rab_ass_test.c index 482b0fc..043b16b 100644 --- a/tests/ranap_rab_ass/ranap_rab_ass_test.c +++ b/tests/ranap_rab_ass/ranap_rab_ass_test.c @@ -55,19 +55,21 @@ void test_ranap_rab_ass_req_decode_encode(void) 0x00, 0x00, 0x00, 0x00, 0x00, 0x40, 0x1f, 0x76, 0x00, 0x00, 0x40, 0x01, 0x00 }; - uint8_t encoded[sizeof(testvec)]; + struct msgb *encoded; rc = ranap_ran_rx_co_decode(talloc_asn1_ctx, &message, testvec, sizeof(testvec)); OSMO_ASSERT(rc == 0); - rc = ranap_rab_ass_req_encode(encoded, sizeof(encoded), &message.msg.raB_AssignmentRequestIEs); - printf("ranap_rab_ass_req_encode rc=%d\n", rc); + encoded = ranap_rab_ass_req_encode(&message.msg.raB_AssignmentRequestIEs); + printf("ranap_rab_ass_req_encode %s\n", encoded ? "ok" : "ERROR"); printf("INPUT: %s\n", osmo_hexdump_nospc(testvec, sizeof(testvec))); - printf("RESULT: %s\n", osmo_hexdump_nospc(encoded, sizeof(encoded))); - OSMO_ASSERT(memcmp(testvec, encoded, sizeof(testvec)) == 0); + printf("RESULT: %s\n", osmo_hexdump_nospc(encoded->data, encoded->len)); + OSMO_ASSERT(encoded->len == sizeof(testvec)); + OSMO_ASSERT(memcmp(testvec, encoded->data, sizeof(testvec)) == 0); ranap_ran_rx_co_free(&message); + msgb_free(encoded); } void test_ranap_rab_ass_resp_decode_encode(void) @@ -158,6 +160,7 @@ void test_ranap_rab_ass_req_ies_replace_inet_addr(void) struct osmo_sockaddr addr; struct osmo_sockaddr_str addr_str; ranap_message message; + struct msgb *encoded; uint8_t rab_id; uint8_t testvec_in[] = { 0x00, 0x00, 0x00, 0x59, 0x00, 0x00, 0x01, 0x00, @@ -210,11 +213,12 @@ void test_ranap_rab_ass_req_ies_replace_inet_addr(void) osmo_sockaddr_str_from_sockaddr(&addr_str, &addr.u.sas); printf("after: addr=%s, port=%u, rab_id=%u\n", addr_str.ip, addr_str.port, rab_id); - rc = ranap_rab_ass_req_encode(testvec_in, sizeof(testvec_in), &message.msg.raB_AssignmentRequestIEs); - OSMO_ASSERT(rc == sizeof(testvec_in)); - OSMO_ASSERT(memcmp(testvec_in, testvec_expected_out, sizeof(testvec_in)) == 0); + encoded = ranap_rab_ass_req_encode(&message.msg.raB_AssignmentRequestIEs); + OSMO_ASSERT(encoded->len == sizeof(testvec_expected_out)); + OSMO_ASSERT(memcmp(encoded->data, testvec_expected_out, encoded->len) == 0); ranap_ran_rx_co_free(&message); + msgb_free(encoded); } void test_ranap_rab_ass_resp_ies_replace_inet_addr(void) diff --git a/tests/ranap_rab_ass/ranap_rab_ass_test.ok b/tests/ranap_rab_ass/ranap_rab_ass_test.ok index 80def36..47fb97f 100644 --- a/tests/ranap_rab_ass/ranap_rab_ass_test.ok +++ b/tests/ranap_rab_ass/ranap_rab_ass_test.ok @@ -1,4 +1,4 @@ -ranap_rab_ass_req_encode rc=93 +ranap_rab_ass_req_encode ok INPUT: 0000005900000100364052000001003500487822cd80102fa7201a2c0000f44c080a028000514000272028140067400000222814003c40000000503d02000227c03500010a0901a200000000000000000000000000401f760000400100 RESULT: 0000005900000100364052000001003500487822cd80102fa7201a2c0000f44c080a028000514000272028140067400000222814003c40000000503d02000227c03500010a0901a200000000000000000000000000401f760000400100 ranap_rab_ass_resp_encode rc=46