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/.
laforge gerrit-no-reply at lists.osmocom.orglaforge has submitted this change and it was merged. ( 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(-) Approvals: Jenkins Builder: Verified laforge: Looks good to me, approved 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 <laforge at gnumonks.org> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge at gnumonks.org> Gerrit-MessageType: merged -------------- next part -------------- An HTML attachment was scrubbed... URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20190912/3536da45/attachment.htm>