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

src/remsim_client.c

@@ -90,7 +90,6 @@ int bankd_read_cb(struct ipa_client_conn *conn, struct msgb *msg)
 		default:
 			break;
 		}
-		msgb_free(msg);
 		break;
 	case IPAC_PROTO_OSMO:
 		if (!he || msgb_l2len(msg) < sizeof(*he))
@@ -109,6 +108,7 @@ int bankd_read_cb(struct ipa_client_conn *conn, struct msgb *msg)
 		goto invalid;
 	}
 
+	msgb_free(msg);
 	return rc;
 
 invalid:

src/rspro_client_fsm.c

@@ -133,7 +133,6 @@ static int srvc_read_cb(struct ipa_client_conn *conn, struct msgb *msg)
 		default:
 			break;
 		}
-		msgb_free(msg);
 		break;
 	case IPAC_PROTO_OSMO:
 		if (!he || msgb_l2len(msg) < sizeof(*he))
@@ -146,7 +145,7 @@ static int srvc_read_cb(struct ipa_client_conn *conn, struct msgb *msg)
 			 * 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 @@ static int srvc_read_cb(struct ipa_client_conn *conn, struct msgb *msg)
 	default:
 		goto invalid;
 	}
+
+	msgb_free(msg);
 	return rc;
 
 invalid:

src/rspro_util.c

@@ -80,7 +80,7 @@ struct msgb *rspro_enc_msg(RsproPDU_t *pdu)
 	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 @@ RsproPDU_t *rspro_dec_msg(struct msgb *msg)
 	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;
 }
 

src/simtrace2-remsim_client.c

@@ -578,7 +578,6 @@ static int bankd_handle_setAtrReq(struct bankd_client *bc, RsproPDU_t *pdu)
 
 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 @@ int bankd_read_cb(struct ipa_client_conn *conn, struct msgb *msg)
 	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: