laforge has uploaded this change for review. (
https://gerrit.osmocom.org/c/osmo-remsim/+/15507
Change subject: rspro_dec_msg: Simplify msgb ownership handling
......................................................................
rspro_dec_msg: Simplify msgb ownership handling
Initially it seemed like a good idea that rspro_dec_msg() takes care
of freeing the input msgb when generating a new decoded output
structure. However, in reality this made the implementation of
every caller more complicated, as it had to treat messages going into
rspro_dec_msg() differently than messages going elsewhere.
Adding to that, not every caller got it right, and the comments were
disagreeing about what happens to msgb ownership in erroneous cases.
Change-Id: I55d5d61922053fd94e2b5a8cdf0d799b29feec98
---
M src/remsim_client.c
M src/rspro_client_fsm.c
M src/rspro_util.c
M src/simtrace2-remsim_client.c
4 files changed, 6 insertions(+), 8 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-remsim refs/changes/07/15507/1
diff --git a/src/remsim_client.c b/src/remsim_client.c
index e73fcc4..0bfc0a4 100644
--- a/src/remsim_client.c
+++ b/src/remsim_client.c
@@ -90,7 +90,6 @@
default:
break;
}
- msgb_free(msg);
break;
case IPAC_PROTO_OSMO:
if (!he || msgb_l2len(msg) < sizeof(*he))
@@ -109,6 +108,7 @@
goto invalid;
}
+ msgb_free(msg);
return rc;
invalid:
diff --git a/src/rspro_client_fsm.c b/src/rspro_client_fsm.c
index d487f32..45cd08e 100644
--- a/src/rspro_client_fsm.c
+++ b/src/rspro_client_fsm.c
@@ -133,7 +133,6 @@
default:
break;
}
- msgb_free(msg);
break;
case IPAC_PROTO_OSMO:
if (!he || msgb_l2len(msg) < sizeof(*he))
@@ -146,7 +145,7 @@
* and unsuccessful cases */
pdu = rspro_dec_msg(msg);
if (!pdu)
- goto invalid;
+ break;
rc = srvc->handle_rx(srvc, pdu);
ASN_STRUCT_FREE(asn_DEF_RsproPDU, pdu);
break;
@@ -157,6 +156,8 @@
default:
goto invalid;
}
+
+ msgb_free(msg);
return rc;
invalid:
diff --git a/src/rspro_util.c b/src/rspro_util.c
index 1e5ce99..5c78b60 100644
--- a/src/rspro_util.c
+++ b/src/rspro_util.c
@@ -80,7 +80,7 @@
return msg;
}
-/* consumes 'msg' _if_ it is successful */
+/* caller must make sure to free msg */
RsproPDU_t *rspro_dec_msg(struct msgb *msg)
{
RsproPDU_t *pdu = NULL;
@@ -91,12 +91,9 @@
if (rval.code != RC_OK) {
fprintf(stderr, "Failed to decode: %d. Consumed %lu of %u
bytes\n",
rval.code, rval.consumed, msgb_length(msg));
- msgb_free(msg);
return NULL;
}
- msgb_free(msg);
-
return pdu;
}
diff --git a/src/simtrace2-remsim_client.c b/src/simtrace2-remsim_client.c
index 094773f..04aa0c6 100644
--- a/src/simtrace2-remsim_client.c
+++ b/src/simtrace2-remsim_client.c
@@ -578,7 +578,6 @@
static int bankd_handle_msg(struct bankd_client *bc, struct msgb *msg)
{
- /* rspro_dec_msg takes ownership of msgb and talloc_free()s it in
successful and unsuccessful case */
RsproPDU_t *pdu = rspro_dec_msg(msg);
if (!pdu) {
LOGPFSML(bc->bankd_fi, LOGL_ERROR, "Error decoding PDU\n");
@@ -628,6 +627,7 @@
LOGPFSML(bc->bankd_fi, LOGL_DEBUG, "Received RSPRO %s\n",
msgb_hexdump(msg));
rc = bankd_handle_msg(bc, msg);
+ msgb_free(msg);
return rc;
invalid:
--
To view, visit https://gerrit.osmocom.org/c/osmo-remsim/+/15507
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-remsim
Gerrit-Branch: master
Gerrit-Change-Id: I55d5d61922053fd94e2b5a8cdf0d799b29feec98
Gerrit-Change-Number: 15507
Gerrit-PatchSet: 1
Gerrit-Owner: laforge <[email protected]>
Gerrit-MessageType: newchange
