This is merely a historical archive of years 2008-2021, before the migration to mailman3.
A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.
lynxis lazus gerrit-no-reply at lists.osmocom.orglynxis lazus has submitted this change. ( https://gerrit.osmocom.org/c/libosmocore/+/20912 )
Change subject: libosmocore: change the memory management of NS2
......................................................................
libosmocore: change the memory management of NS2
Until now NS2 always free'd it's own memory. Even when the msg
was sent as primitive to the upper layer.
Change the memory ownership when sending a primitive to the upper layer.
The upper layer has to free the msg buffer.
Merge together with: I180433735bfbb3375c41318d7a7709d5845199ba (osmo-pcu)
Change-Id: Id844d7acbcab102a7dc472d608a5e97a748ecb43
---
M src/gb/gprs_ns2.c
M src/gb/gprs_ns2_udp.c
M src/gb/gprs_ns2_vc_fsm.c
3 files changed, 34 insertions(+), 16 deletions(-)
Approvals:
laforge: Looks good to me, approved
Jenkins Builder: Verified
diff --git a/src/gb/gprs_ns2.c b/src/gb/gprs_ns2.c
index ffa3135..403b8b7 100644
--- a/src/gb/gprs_ns2.c
+++ b/src/gb/gprs_ns2.c
@@ -1001,7 +1001,7 @@
/*! Create a new GPRS NS instance
* \param[in] ctx a talloc context to allocate NS instance from
- * \param[in] cb Call-back function for dispatching primitives to the user
+ * \param[in] cb Call-back function for dispatching primitives to the user. The Call-back must free all msgb* given in the primitive.
* \param[in] cb_data transparent user data passed to Call-back
* \returns dynamically allocated gprs_ns_inst; NULL on error */
struct gprs_ns2_inst *gprs_ns2_instantiate(void *ctx, osmo_prim_cb cb, void *cb_data)
diff --git a/src/gb/gprs_ns2_udp.c b/src/gb/gprs_ns2_udp.c
index 2cc1006..0676314 100644
--- a/src/gb/gprs_ns2_udp.c
+++ b/src/gb/gprs_ns2_udp.c
@@ -206,7 +206,7 @@
static int handle_nsip_read(struct osmo_fd *bfd)
{
- int rc;
+ int rc = 0;
int error = 0;
struct gprs_ns2_vc_bind *bind = bfd->data;
struct osmo_sockaddr saddr;
@@ -240,10 +240,10 @@
}
}
- rc = ns2_recv_vc(nsvc, msg);
+ return ns2_recv_vc(nsvc, msg);
+
out:
msgb_free(msg);
-
return rc;
}
diff --git a/src/gb/gprs_ns2_vc_fsm.c b/src/gb/gprs_ns2_vc_fsm.c
index 451437f..4fa53c1 100644
--- a/src/gb/gprs_ns2_vc_fsm.c
+++ b/src/gb/gprs_ns2_vc_fsm.c
@@ -459,8 +459,10 @@
struct osmo_gprs_ns2_prim nsp = {};
uint16_t bvci;
- if (msgb_l2len(msg) < sizeof(*nsh) + 3)
+ if (msgb_l2len(msg) < sizeof(*nsh) + 3) {
+ msgb_free(msg);
return;
+ }
/* TODO: 7.1: For an IP sub-network, an NS-UNITDATA PDU
* for a PTP BVC may indicate a request to change the IP endpoint
@@ -488,6 +490,7 @@
{
struct gprs_ns2_vc_priv *priv = fi->priv;
struct gprs_ns2_inst *nsi = ns_inst_from_fi(fi);
+ struct msgb *msg = data;
switch (event) {
case GPRS_NS2_EV_RESET:
@@ -520,22 +523,31 @@
recv_test_procedure(fi);
break;
case GPRS_NS2_EV_UNITDATA:
+ /* UNITDATA has to handle the release of msg.
+ * If send upwards (gprs_ns2_recv_unitdata) it must NOT free
+ * the msg, the upper layer has to do it.
+ * Otherwise the msg must be freed.
+ */
switch (fi->state) {
case GPRS_NS2_ST_BLOCKED:
/* 7.2.1: the BLOCKED_ACK might be lost */
- if (priv->initiater)
- gprs_ns2_recv_unitdata(fi, data);
- else
- ns2_tx_status(priv->nsvc,
- NS_CAUSE_NSVC_BLOCKED,
- 0, data);
+ if (priv->initiater) {
+ gprs_ns2_recv_unitdata(fi, msg);
+ return;
+ }
+
+ ns2_tx_status(priv->nsvc,
+ NS_CAUSE_NSVC_BLOCKED,
+ 0, msg);
break;
/* ALIVE can receive UNITDATA if the ALIVE_ACK is lost */
case GPRS_NS2_ST_ALIVE:
case GPRS_NS2_ST_UNBLOCKED:
- gprs_ns2_recv_unitdata(fi, data);
- break;
+ gprs_ns2_recv_unitdata(fi, msg);
+ return;
}
+
+ msgb_free(msg);
break;
}
}
@@ -613,6 +625,7 @@
{
struct gprs_ns_hdr *nsh = (struct gprs_ns_hdr *) msg->l2h;
struct osmo_fsm_inst *fi = nsvc->fi;
+ int rc = 0;
uint8_t cause;
/* TODO: 7.2: on UNBLOCK/BLOCK: check if NS-VCI is correct,
@@ -622,7 +635,8 @@
if (gprs_ns2_validate(nsvc, nsh->pdu_type, msg, tp, &cause)) {
if (nsh->pdu_type != NS_PDUT_STATUS) {
- return ns2_tx_status(nsvc, cause, 0, msg);
+ rc = ns2_tx_status(nsvc, cause, 0, msg);
+ goto out;
}
}
@@ -652,15 +666,19 @@
osmo_fsm_inst_dispatch(fi, GPRS_NS2_EV_ALIVE_ACK, tp);
break;
case NS_PDUT_UNITDATA:
+ /* UNITDATA have to free msg because it might send the msg layer upwards */
osmo_fsm_inst_dispatch(fi, GPRS_NS2_EV_UNITDATA, msg);
- break;
+ return 0;
default:
LOGP(DLNS, LOGL_ERROR, "NSEI=%u Rx unknown NS PDU type %s\n", nsvc->nse->nsei,
get_value_string(gprs_ns_pdu_strings, nsh->pdu_type));
return -EINVAL;
}
- return 0;
+out:
+ msgb_free(msg);
+
+ return rc;
}
/*! is the given NS-VC unblocked? */
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/20912
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: Id844d7acbcab102a7dc472d608a5e97a748ecb43
Gerrit-Change-Number: 20912
Gerrit-PatchSet: 5
Gerrit-Owner: lynxis lazus <lynxis at fe80.eu>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge at osmocom.org>
Gerrit-Reviewer: lynxis lazus <lynxis at fe80.eu>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20201103/ac9d1136/attachment.htm>