neels has uploaded this change for review. (
https://gerrit.osmocom.org/c/osmo-mgw/+/32162
)
Change subject: mgcp_client: simpler error handling
......................................................................
mgcp_client: simpler error handling
add_sdp(), add_lco():
- do not msgb_free() within these functions. Just return error, and move
the msgb_free() to the caler.
- when failing to write to the msgb, directly return error.
Change-Id: I904d56f56053952c2ebbbe5dca744fafa32b333e
---
M src/libosmo-mgcp-client/mgcp_client.c
1 file changed, 56 insertions(+), 54 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/62/32162/1
diff --git a/src/libosmo-mgcp-client/mgcp_client.c
b/src/libosmo-mgcp-client/mgcp_client.c
index 670910b..fb4c17a 100644
--- a/src/libosmo-mgcp-client/mgcp_client.c
+++ b/src/libosmo-mgcp-client/mgcp_client.c
@@ -1131,47 +1131,43 @@
#define MGCP_AUEP_MANDATORY (MGCP_MSG_PRESENCE_ENDPOINT)
#define MGCP_RSIP_MANDATORY 0 /* none */
+#define MSGB_PRINTF_OR_RET(msg, FMT, ARGS...) \
+ if (msgb_printf(msg, FMT, ##ARGS) != 0) { \
+ LOGP(DLMGCP, LOGL_ERROR, "Message buffer to small, can not generate MGCP/SDP
message\n"); \
+ return -ENOBUFS; \
+ }
+
/* Helper function for mgcp_msg_gen(): Add LCO information to MGCP message */
static int add_lco(struct msgb *msg, struct mgcp_msg *mgcp_msg)
{
unsigned int i;
- int rc = 0;
const char *codec;
unsigned int pt;
- rc |= msgb_printf(msg, "L:");
+ MSGB_PRINTF_OR_RET(msg, "L:");
if (mgcp_msg->ptime)
- rc |= msgb_printf(msg, " p:%u,", mgcp_msg->ptime);
+ MSGB_PRINTF_OR_RET(msg, " p:%u,", mgcp_msg->ptime);
if (mgcp_msg->codecs_len) {
- rc |= msgb_printf(msg, " a:");
+ MSGB_PRINTF_OR_RET(msg, " a:");
for (i = 0; i < mgcp_msg->codecs_len; i++) {
pt = mgcp_msg->codecs[i];
codec = get_value_string_or_null(osmo_mgcpc_codec_names, pt);
/* Note: Use codec descriptors from enum mgcp_codecs
* in mgcp_client only! */
- if (!codec) {
- msgb_free(msg);
+ if (!codec)
return -EINVAL;
- }
- rc |= msgb_printf(msg, "%s", extract_codec_name(codec));
+ MSGB_PRINTF_OR_RET(msg, "%s", extract_codec_name(codec));
if (i < mgcp_msg->codecs_len - 1)
- rc |= msgb_printf(msg, ";");
+ MSGB_PRINTF_OR_RET(msg, ";");
}
- rc |= msgb_printf(msg, ",");
+ MSGB_PRINTF_OR_RET(msg, ",");
}
- rc |= msgb_printf(msg, " nt:IN\r\n");
-
- if (rc != 0) {
- LOGP(DLMGCP, LOGL_ERROR,
- "message buffer to small, can not generate MGCP message (LCO)\n");
- msgb_free(msg);
- return -ENOBUFS;
- }
+ MSGB_PRINTF_OR_RET(msg, " nt:IN\r\n");
return 0;
}
@@ -1179,43 +1175,37 @@
static int add_sdp(struct msgb *msg, struct mgcp_msg *mgcp_msg, struct mgcp_client
*mgcp)
{
unsigned int i;
- int rc = 0;
char local_ip[INET6_ADDRSTRLEN];
int local_ip_family, audio_ip_family;
const char *codec;
unsigned int pt;
/* Add separator to mark the beginning of the SDP block */
- rc |= msgb_printf(msg, "\r\n");
+ MSGB_PRINTF_OR_RET(msg, "\r\n");
/* Add SDP protocol version */
- rc |= msgb_printf(msg, "v=0\r\n");
+ MSGB_PRINTF_OR_RET(msg, "v=0\r\n");
/* Determine local IP-Address */
if (osmo_sock_local_ip(local_ip, mgcp->actual.remote_addr) < 0) {
LOGPMGW(mgcp, LOGL_ERROR,
"Could not determine local IP-Address!\n");
- msgb_free(msg);
return -EINVAL;
}
local_ip_family = osmo_ip_str_type(local_ip);
- if (local_ip_family == AF_UNSPEC) {
- msgb_free(msg);
+ if (local_ip_family == AF_UNSPEC)
return -EINVAL;
- }
audio_ip_family = osmo_ip_str_type(mgcp_msg->audio_ip);
- if (audio_ip_family == AF_UNSPEC) {
- msgb_free(msg);
+ if (audio_ip_family == AF_UNSPEC)
return -EINVAL;
- }
/* Add owner/creator (SDP) */
- rc |= msgb_printf(msg, "o=- %x 23 IN IP%c %s\r\n", mgcp_msg->call_id,
+ MSGB_PRINTF_OR_RET(msg, "o=- %x 23 IN IP%c %s\r\n", mgcp_msg->call_id,
local_ip_family == AF_INET6 ? '6' : '4',
local_ip);
/* Add session name (none) */
- rc |= msgb_printf(msg, "s=-\r\n");
+ MSGB_PRINTF_OR_RET(msg, "s=-\r\n");
/* Add RTP address and port */
if (mgcp_msg->audio_port == 0) {
@@ -1230,20 +1220,20 @@
msgb_free(msg);
return -EINVAL;
}
- rc |= msgb_printf(msg, "c=IN IP%c %s\r\n",
+ MSGB_PRINTF_OR_RET(msg, "c=IN IP%c %s\r\n",
audio_ip_family == AF_INET6 ? '6' : '4',
mgcp_msg->audio_ip);
/* Add time description, active time (SDP) */
- rc |= msgb_printf(msg, "t=0 0\r\n");
+ MSGB_PRINTF_OR_RET(msg, "t=0 0\r\n");
- rc |= msgb_printf(msg, "m=audio %u RTP/AVP", mgcp_msg->audio_port);
+ MSGB_PRINTF_OR_RET(msg, "m=audio %u RTP/AVP", mgcp_msg->audio_port);
for (i = 0; i < mgcp_msg->codecs_len; i++) {
pt = map_codec_to_pt(mgcp_msg->ptmap, mgcp_msg->ptmap_len,
mgcp_msg->codecs[i]);
- rc |= msgb_printf(msg, " %u", pt);
+ MSGB_PRINTF_OR_RET(msg, " %u", pt);
}
- rc |= msgb_printf(msg, "\r\n");
+ MSGB_PRINTF_OR_RET(msg, "\r\n");
/* Add optional codec parameters (fmtp) */
if (mgcp_msg->param_present) {
@@ -1253,9 +1243,9 @@
continue;
pt = map_codec_to_pt(mgcp_msg->ptmap, mgcp_msg->ptmap_len,
mgcp_msg->codecs[i]);
if (mgcp_msg->param.amr_octet_aligned_present &&
mgcp_msg->param.amr_octet_aligned)
- rc |= msgb_printf(msg, "a=fmtp:%u octet-align=1\r\n", pt);
+ MSGB_PRINTF_OR_RET(msg, "a=fmtp:%u octet-align=1\r\n", pt);
else if (mgcp_msg->param.amr_octet_aligned_present &&
!mgcp_msg->param.amr_octet_aligned)
- rc |= msgb_printf(msg, "a=fmtp:%u octet-align=0\r\n", pt);
+ MSGB_PRINTF_OR_RET(msg, "a=fmtp:%u octet-align=0\r\n", pt);
}
}
@@ -1275,18 +1265,12 @@
return -EINVAL;
}
- rc |= msgb_printf(msg, "a=rtpmap:%u %s\r\n", pt, codec);
+ MSGB_PRINTF_OR_RET(msg, "a=rtpmap:%u %s\r\n", pt, codec);
}
}
if (mgcp_msg->ptime)
- rc |= msgb_printf(msg, "a=ptime:%u\r\n", mgcp_msg->ptime);
-
- if (rc != 0) {
- LOGPMGW(mgcp, LOGL_ERROR, "Message buffer to small, can not generate MGCP message
(SDP)\n");
- msgb_free(msg);
- return -ENOBUFS;
- }
+ MSGB_PRINTF_OR_RET(msg, "a=ptime:%u\r\n", mgcp_msg->ptime);
return 0;
}
@@ -1311,23 +1295,23 @@
switch (mgcp_msg->verb) {
case MGCP_VERB_CRCX:
mandatory_mask = MGCP_CRCX_MANDATORY;
- rc |= msgb_printf(msg, "CRCX %u", trans_id);
+ MSGB_PRINTF_OR_RET(msg, "CRCX %u", trans_id);
break;
case MGCP_VERB_MDCX:
mandatory_mask = MGCP_MDCX_MANDATORY;
- rc |= msgb_printf(msg, "MDCX %u", trans_id);
+ MSGB_PRINTF_OR_RET(msg, "MDCX %u", trans_id);
break;
case MGCP_VERB_DLCX:
mandatory_mask = MGCP_DLCX_MANDATORY;
- rc |= msgb_printf(msg, "DLCX %u", trans_id);
+ MSGB_PRINTF_OR_RET(msg, "DLCX %u", trans_id);
break;
case MGCP_VERB_AUEP:
mandatory_mask = MGCP_AUEP_MANDATORY;
- rc |= msgb_printf(msg, "AUEP %u", trans_id);
+ MSGB_PRINTF_OR_RET(msg, "AUEP %u", trans_id);
break;
case MGCP_VERB_RSIP:
mandatory_mask = MGCP_RSIP_MANDATORY;
- rc |= msgb_printf(msg, "RSIP %u", trans_id);
+ MSGB_PRINTF_OR_RET(msg, "RSIP %u", trans_id);
break;
default:
LOGPMGW(mgcp, LOGL_ERROR, "Invalid command verb, can not generate MGCP
message\n");
@@ -1359,15 +1343,15 @@
return NULL;
}
- rc |= msgb_printf(msg, " %s", mgcp_msg->endpoint);
+ MSGB_PRINTF_OR_RET(msg, " %s", mgcp_msg->endpoint);
}
/* Add protocol version */
- rc |= msgb_printf(msg, " MGCP 1.0\r\n");
+ MSGB_PRINTF_OR_RET(msg, " MGCP 1.0\r\n");
/* Add call id */
if (mgcp_msg->presence & MGCP_MSG_PRESENCE_CALL_ID)
- rc |= msgb_printf(msg, "C: %x\r\n", mgcp_msg->call_id);
+ MSGB_PRINTF_OR_RET(msg, "C: %x\r\n", mgcp_msg->call_id);
/* Add connection id */
if (mgcp_msg->presence & MGCP_MSG_PRESENCE_CONN_ID) {
@@ -1376,7 +1360,7 @@
msgb_free(msg);
return NULL;
}
- rc |= msgb_printf(msg, "I: %s\r\n", mgcp_msg->conn_id);
+ MSGB_PRINTF_OR_RET(msg, "I: %s\r\n", mgcp_msg->conn_id);
}
/* Using SDP makes sense when a valid IP/Port combination is specifiec,
@@ -1391,6 +1375,7 @@
|| mgcp_msg->verb == MGCP_VERB_MDCX)) {
if (add_lco(msg, mgcp_msg) < 0) {
LOGPMGW(mgcp, LOGL_ERROR, "Failed to add LCO, can not generate MGCP
message\n");
+ msgb_free(msg);
return NULL;
}
}
@@ -1429,6 +1414,7 @@
|| mgcp_msg->verb == MGCP_VERB_MDCX)) {
if (add_sdp(msg, mgcp_msg, mgcp) < 0) {
LOGPMGW(mgcp, LOGL_ERROR, "Failed to add SDP, can not generate MGCP
message\n");
+ msgb_free(msg);
return NULL;
}
}
--
To view, visit
https://gerrit.osmocom.org/c/osmo-mgw/+/32162
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Change-Id: I904d56f56053952c2ebbbe5dca744fafa32b333e
Gerrit-Change-Number: 32162
Gerrit-PatchSet: 1
Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de>
Gerrit-MessageType: newchange