mgcp_client: fix error handling in mgcp message generation
The functions add_lco and add_sdp assert when the codec string can not be generated. This is the case when an unexpected codec is addressed in the input parameter mgcp_msg for mgcp_msg_gen(). Even though the API user is expected only to use the codec identifiers in mgcp_client.h the check should not be done with an assert. Instead mgcp_msg_gen() should just return NULL imediately. Also all generation functions should not use magic numbers as return codes. Instead constants from errno.h should be used. It is also problematic that the return codes from msgb_printf are added up. Depending. It makes more sense to use an OR operator since msgb_printf only returns 0 or -EINVAL, so the end result will be -EINVAL if one or more msgb_printf fail and not just a random negative value. Change-Id: Ibb788343e0bec9c0eaf33e6e4727d4d36c100017 Related: OS#5119
This commit is contained in:
parent
eb984bd630
commit
7d86d4c523
|
@ -1105,30 +1105,40 @@ static int add_lco(struct msgb *msg, struct mgcp_msg *mgcp_msg)
|
|||
const char *codec;
|
||||
unsigned int pt;
|
||||
|
||||
rc += msgb_printf(msg, "L:");
|
||||
rc |= msgb_printf(msg, "L:");
|
||||
|
||||
if (mgcp_msg->ptime)
|
||||
rc += msgb_printf(msg, " p:%u,", mgcp_msg->ptime);
|
||||
rc |= msgb_printf(msg, " p:%u,", mgcp_msg->ptime);
|
||||
|
||||
if (mgcp_msg->codecs_len) {
|
||||
rc += msgb_printf(msg, " a:");
|
||||
rc |= msgb_printf(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! */
|
||||
OSMO_ASSERT(codec);
|
||||
rc += msgb_printf(msg, "%s", extract_codec_name(codec));
|
||||
if (!codec) {
|
||||
msgb_free(msg);
|
||||
return -EINVAL;
|
||||
}
|
||||
|
||||
rc |= msgb_printf(msg, "%s", extract_codec_name(codec));
|
||||
if (i < mgcp_msg->codecs_len - 1)
|
||||
rc += msgb_printf(msg, ";");
|
||||
rc |= msgb_printf(msg, ";");
|
||||
}
|
||||
rc += msgb_printf(msg, ",");
|
||||
rc |= msgb_printf(msg, ",");
|
||||
}
|
||||
|
||||
rc += msgb_printf(msg, " nt:IN\r\n");
|
||||
rc |= msgb_printf(msg, " nt:IN\r\n");
|
||||
|
||||
return rc;
|
||||
if (rc != 0) {
|
||||
LOGP(DLMGCP, LOGL_ERROR,
|
||||
"message buffer to small, can not generate MGCP message (LCO)\n");
|
||||
msgb_free(msg);
|
||||
return -ENOBUFS;
|
||||
}
|
||||
return 0;
|
||||
}
|
||||
|
||||
/* Helper function for mgcp_msg_gen(): Add SDP information to MGCP message */
|
||||
|
@ -1142,64 +1152,64 @@ static int add_sdp(struct msgb *msg, struct mgcp_msg *mgcp_msg, struct mgcp_clie
|
|||
unsigned int pt;
|
||||
|
||||
/* Add separator to mark the beginning of the SDP block */
|
||||
rc += msgb_printf(msg, "\r\n");
|
||||
rc |= msgb_printf(msg, "\r\n");
|
||||
|
||||
/* Add SDP protocol version */
|
||||
rc += msgb_printf(msg, "v=0\r\n");
|
||||
rc |= msgb_printf(msg, "v=0\r\n");
|
||||
|
||||
/* Determine local IP-Address */
|
||||
if (osmo_sock_local_ip(local_ip, mgcp->actual.remote_addr) < 0) {
|
||||
LOGP(DLMGCP, LOGL_ERROR,
|
||||
"Could not determine local IP-Address!\n");
|
||||
msgb_free(msg);
|
||||
return -2;
|
||||
return -EINVAL;
|
||||
}
|
||||
local_ip_family = osmo_ip_str_type(local_ip);
|
||||
if (local_ip_family == AF_UNSPEC) {
|
||||
msgb_free(msg);
|
||||
return -2;
|
||||
return -EINVAL;
|
||||
}
|
||||
audio_ip_family = osmo_ip_str_type(mgcp_msg->audio_ip);
|
||||
if (audio_ip_family == AF_UNSPEC) {
|
||||
msgb_free(msg);
|
||||
return -2;
|
||||
return -EINVAL;
|
||||
}
|
||||
|
||||
/* Add owner/creator (SDP) */
|
||||
rc += msgb_printf(msg, "o=- %x 23 IN IP%c %s\r\n", mgcp_msg->call_id,
|
||||
rc |= msgb_printf(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");
|
||||
rc |= msgb_printf(msg, "s=-\r\n");
|
||||
|
||||
/* Add RTP address and port */
|
||||
if (mgcp_msg->audio_port == 0) {
|
||||
LOGP(DLMGCP, LOGL_ERROR,
|
||||
"Invalid port number, can not generate MGCP message\n");
|
||||
msgb_free(msg);
|
||||
return -2;
|
||||
return -EINVAL;
|
||||
}
|
||||
if (strlen(mgcp_msg->audio_ip) <= 0) {
|
||||
LOGP(DLMGCP, LOGL_ERROR,
|
||||
"Empty ip address, can not generate MGCP message\n");
|
||||
msgb_free(msg);
|
||||
return -2;
|
||||
return -EINVAL;
|
||||
}
|
||||
rc += msgb_printf(msg, "c=IN IP%c %s\r\n",
|
||||
rc |= msgb_printf(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");
|
||||
rc |= msgb_printf(msg, "t=0 0\r\n");
|
||||
|
||||
rc += msgb_printf(msg, "m=audio %u RTP/AVP", mgcp_msg->audio_port);
|
||||
rc |= msgb_printf(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);
|
||||
rc |= msgb_printf(msg, " %u", pt);
|
||||
|
||||
}
|
||||
rc += msgb_printf(msg, "\r\n");
|
||||
rc |= msgb_printf(msg, "\r\n");
|
||||
|
||||
/* Add optional codec parameters (fmtp) */
|
||||
if (mgcp_msg->param_present) {
|
||||
|
@ -1209,9 +1219,9 @@ static int add_sdp(struct msgb *msg, struct mgcp_msg *mgcp_msg, struct mgcp_clie
|
|||
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);
|
||||
rc |= msgb_printf(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);
|
||||
rc |= msgb_printf(msg, "a=fmtp:%u octet-align=0\r\n", pt);
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -1226,16 +1236,26 @@ static int add_sdp(struct msgb *msg, struct mgcp_msg *mgcp_msg, struct mgcp_clie
|
|||
|
||||
/* Note: Use codec descriptors from enum mgcp_codecs
|
||||
* in mgcp_client only! */
|
||||
OSMO_ASSERT(codec);
|
||||
if (!codec) {
|
||||
msgb_free(msg);
|
||||
return -EINVAL;
|
||||
}
|
||||
|
||||
rc += msgb_printf(msg, "a=rtpmap:%u %s\r\n", pt, codec);
|
||||
rc |= msgb_printf(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);
|
||||
rc |= msgb_printf(msg, "a=ptime:%u\r\n", mgcp_msg->ptime);
|
||||
|
||||
return rc;
|
||||
if (rc != 0) {
|
||||
LOGP(DLMGCP, LOGL_ERROR,
|
||||
"message buffer to small, can not generate MGCP message (SDP)\n");
|
||||
msgb_free(msg);
|
||||
return -ENOBUFS;
|
||||
}
|
||||
|
||||
return 0;
|
||||
}
|
||||
|
||||
/*! Generate an MGCP message
|
||||
|
@ -1248,7 +1268,6 @@ struct msgb *mgcp_msg_gen(struct mgcp_client *mgcp, struct mgcp_msg *mgcp_msg)
|
|||
uint32_t mandatory_mask;
|
||||
struct msgb *msg = msgb_alloc_headroom(4096, 128, "MGCP tx");
|
||||
int rc = 0;
|
||||
int rc_sdp;
|
||||
bool use_sdp = false;
|
||||
char buf[32];
|
||||
|
||||
|
@ -1259,23 +1278,23 @@ struct msgb *mgcp_msg_gen(struct mgcp_client *mgcp, struct mgcp_msg *mgcp_msg)
|
|||
switch (mgcp_msg->verb) {
|
||||
case MGCP_VERB_CRCX:
|
||||
mandatory_mask = MGCP_CRCX_MANDATORY;
|
||||
rc += msgb_printf(msg, "CRCX %u", trans_id);
|
||||
rc |= msgb_printf(msg, "CRCX %u", trans_id);
|
||||
break;
|
||||
case MGCP_VERB_MDCX:
|
||||
mandatory_mask = MGCP_MDCX_MANDATORY;
|
||||
rc += msgb_printf(msg, "MDCX %u", trans_id);
|
||||
rc |= msgb_printf(msg, "MDCX %u", trans_id);
|
||||
break;
|
||||
case MGCP_VERB_DLCX:
|
||||
mandatory_mask = MGCP_DLCX_MANDATORY;
|
||||
rc += msgb_printf(msg, "DLCX %u", trans_id);
|
||||
rc |= msgb_printf(msg, "DLCX %u", trans_id);
|
||||
break;
|
||||
case MGCP_VERB_AUEP:
|
||||
mandatory_mask = MGCP_AUEP_MANDATORY;
|
||||
rc += msgb_printf(msg, "AUEP %u", trans_id);
|
||||
rc |= msgb_printf(msg, "AUEP %u", trans_id);
|
||||
break;
|
||||
case MGCP_VERB_RSIP:
|
||||
mandatory_mask = MGCP_RSIP_MANDATORY;
|
||||
rc += msgb_printf(msg, "RSIP %u", trans_id);
|
||||
rc |= msgb_printf(msg, "RSIP %u", trans_id);
|
||||
break;
|
||||
default:
|
||||
LOGP(DLMGCP, LOGL_ERROR,
|
||||
|
@ -1309,15 +1328,15 @@ struct msgb *mgcp_msg_gen(struct mgcp_client *mgcp, struct mgcp_msg *mgcp_msg)
|
|||
return NULL;
|
||||
}
|
||||
|
||||
rc += msgb_printf(msg, " %s", mgcp_msg->endpoint);
|
||||
rc |= msgb_printf(msg, " %s", mgcp_msg->endpoint);
|
||||
}
|
||||
|
||||
/* Add protocol version */
|
||||
rc += msgb_printf(msg, " MGCP 1.0\r\n");
|
||||
rc |= msgb_printf(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);
|
||||
rc |= msgb_printf(msg, "C: %x\r\n", mgcp_msg->call_id);
|
||||
|
||||
/* Add connection id */
|
||||
if (mgcp_msg->presence & MGCP_MSG_PRESENCE_CONN_ID) {
|
||||
|
@ -1327,7 +1346,7 @@ struct msgb *mgcp_msg_gen(struct mgcp_client *mgcp, struct mgcp_msg *mgcp_msg)
|
|||
msgb_free(msg);
|
||||
return NULL;
|
||||
}
|
||||
rc += msgb_printf(msg, "I: %s\r\n", mgcp_msg->conn_id);
|
||||
rc |= msgb_printf(msg, "I: %s\r\n", mgcp_msg->conn_id);
|
||||
}
|
||||
|
||||
/* Using SDP makes sense when a valid IP/Port combination is specifiec,
|
||||
|
@ -1339,19 +1358,21 @@ struct msgb *mgcp_msg_gen(struct mgcp_client *mgcp, struct mgcp_msg *mgcp_msg)
|
|||
/* Add local connection options (LCO) */
|
||||
if (!use_sdp
|
||||
&& (mgcp_msg->verb == MGCP_VERB_CRCX
|
||||
|| mgcp_msg->verb == MGCP_VERB_MDCX))
|
||||
rc += add_lco(msg, mgcp_msg);
|
||||
|| mgcp_msg->verb == MGCP_VERB_MDCX)) {
|
||||
if (add_lco(msg, mgcp_msg) < 0)
|
||||
return NULL;
|
||||
}
|
||||
|
||||
/* Add mode */
|
||||
if (mgcp_msg->presence & MGCP_MSG_PRESENCE_CONN_MODE)
|
||||
rc +=
|
||||
rc |=
|
||||
msgb_printf(msg, "M: %s\r\n",
|
||||
mgcp_client_cmode_name(mgcp_msg->conn_mode));
|
||||
|
||||
/* Add X-Osmo-IGN */
|
||||
if ((mgcp_msg->presence & MGCP_MSG_PRESENCE_X_OSMO_IGN)
|
||||
&& (mgcp_msg->x_osmo_ign != 0))
|
||||
rc +=
|
||||
rc |=
|
||||
msgb_printf(msg, MGCP_X_OSMO_IGN_HEADER "%s\r\n",
|
||||
mgcp_msg->x_osmo_ign & MGCP_X_OSMO_IGN_CALLID ? " C": "");
|
||||
|
||||
|
@ -1365,7 +1386,7 @@ struct msgb *mgcp_msg_gen(struct mgcp_client *mgcp, struct mgcp_msg *mgcp_msg)
|
|||
return NULL;
|
||||
}
|
||||
snprintf(buf, sizeof(buf), " %d", mgcp_msg->x_osmo_osmux_cid);
|
||||
rc +=
|
||||
rc |=
|
||||
msgb_printf(msg, MGCP_X_OSMO_OSMUX_HEADER "%s\r\n",
|
||||
mgcp_msg->x_osmo_osmux_cid == -1 ? " *": buf);
|
||||
}
|
||||
|
@ -1375,10 +1396,8 @@ struct msgb *mgcp_msg_gen(struct mgcp_client *mgcp, struct mgcp_msg *mgcp_msg)
|
|||
if (use_sdp
|
||||
&& (mgcp_msg->verb == MGCP_VERB_CRCX
|
||||
|| mgcp_msg->verb == MGCP_VERB_MDCX)) {
|
||||
rc_sdp = add_sdp(msg, mgcp_msg, mgcp);
|
||||
if (rc_sdp == -2)
|
||||
if (add_sdp(msg, mgcp_msg, mgcp) < 0)
|
||||
return NULL;
|
||||
rc += rc_sdp;
|
||||
}
|
||||
|
||||
if (rc != 0) {
|
||||
|
|
|
@ -1,5 +1,5 @@
|
|||
DLMGCP MGCP client: using endpoint domain '@mgw'
|
||||
DLMGCP message buffer to small, can not generate MGCP message
|
||||
DLMGCP message buffer to small, can not generate MGCP message (SDP)
|
||||
|
||||
test_mgcp_client_cancel():
|
||||
DLMGCP MGCP client: using endpoint domain '@mgw'
|
||||
|
|
Loading…
Reference in New Issue