Change in osmo-mgw[master]: libosmo-mgcp: cleanup audio codex alloc

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.org
Mon Sep 13 19:05:51 UTC 2021


laforge has submitted this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/25421 )

Change subject: libosmo-mgcp: cleanup audio codex alloc
......................................................................

libosmo-mgcp: cleanup audio codex alloc

No need to complicate audio codes with pointers, our "usual" string is
barely larger than a poointer, five times smaller than a talloc header,
and most importantly not really optional anyway...

Change-Id: Icc41643050a5e1ca3c66f307d60b6911ba1b8032
---
M include/osmocom/mgcp/mgcp_network.h
M src/libosmo-mgcp/mgcp_codec.c
M src/libosmo-mgcp/mgcp_sdp.c
3 files changed, 18 insertions(+), 22 deletions(-)

Approvals:
  Jenkins Builder: Verified
  laforge: Looks good to me, approved
  pespin: Looks good to me, but someone else must approve



diff --git a/include/osmocom/mgcp/mgcp_network.h b/include/osmocom/mgcp/mgcp_network.h
index d6d90dd..b9cf5e3 100644
--- a/include/osmocom/mgcp/mgcp_network.h
+++ b/include/osmocom/mgcp/mgcp_network.h
@@ -81,8 +81,8 @@
 	uint32_t frame_duration_den;
 
 	int payload_type;
-	char *audio_name;
-	char *subtype_name;
+	char audio_name[64];
+	char subtype_name[64];
 
 	bool param_present;
 	struct mgcp_codec_param param;
diff --git a/src/libosmo-mgcp/mgcp_codec.c b/src/libosmo-mgcp/mgcp_codec.c
index f237e38..06dcedc 100644
--- a/src/libosmo-mgcp/mgcp_codec.c
+++ b/src/libosmo-mgcp/mgcp_codec.c
@@ -89,15 +89,13 @@
 		.frame_duration_den = DEFAULT_RTP_AUDIO_FRAME_DUR_DEN,
 		.rate = DEFAULT_RTP_AUDIO_DEFAULT_RATE,
 		.channels = DEFAULT_RTP_AUDIO_DEFAULT_CHANNELS,
+		.subtype_name = "",
+		.audio_name = "",
 	};
 }
 
 static void codec_free(struct mgcp_rtp_codec *codec)
 {
-	if (codec->subtype_name)
-		talloc_free(codec->subtype_name);
-	if (codec->audio_name)
-		talloc_free(codec->audio_name);
 	*codec = (struct mgcp_rtp_codec){};
 }
 
@@ -124,10 +122,8 @@
 {
 	int rate;
 	int channels;
-	char audio_codec[64];
 	struct mgcp_rtp_codec *codec;
 	unsigned int pt_offset = conn->end.codecs_assigned;
-	void *ctx = conn->conn;
 
 	/* The amount of codecs we can store is limited, make sure we do not
 	 * overrun this limit. */
@@ -160,16 +156,16 @@
 	if (!audio_name) {
 		switch (payload_type) {
 		case 0:
-			audio_name = talloc_strdup(ctx, "PCMU/8000/1");
+			strcpy(codec->audio_name, "PCMU/8000/1");
 			break;
 		case 3:
-			audio_name = talloc_strdup(ctx, "GSM/8000/1");
+			strcpy(codec->audio_name, "GSM/8000/1");
 			break;
 		case 8:
-			audio_name = talloc_strdup(ctx, "PCMA/8000/1");
+			strcpy(codec->audio_name, "PCMA/8000/1");
 			break;
 		case 18:
-			audio_name = talloc_strdup(ctx, "G729/8000/1");
+			strcpy(codec->audio_name, "G729/8000/1");
 			break;
 		default:
 			/* The given payload type is not known to us, or it
@@ -179,36 +175,36 @@
 			     payload_type);
 			goto error;
 		}
+	} else {
+		strncpy(codec->audio_name, audio_name, sizeof(codec->audio_name));
 	}
 
 	/* Now we extract the codec subtype name, rate and channels. The latter
 	 * two are optional. If they are not present we use the safe defaults
 	 * above. */
-	if (strlen(audio_name) >= sizeof(audio_codec)) {
-		LOGP(DLMGCP, LOGL_ERROR, "Audio codec too long: %s\n", osmo_quote_str(audio_name, -1));
+	if (strlen(codec->audio_name) >= sizeof(codec->subtype_name)) {
+		LOGP(DLMGCP, LOGL_ERROR, "Audio codec too long: %s\n", osmo_quote_str(codec->audio_name, -1));
 		goto error;
 	}
 	channels = DEFAULT_RTP_AUDIO_DEFAULT_CHANNELS;
 	rate = DEFAULT_RTP_AUDIO_DEFAULT_RATE;
-	if (sscanf(audio_name, "%63[^/]/%d/%d", audio_codec, &rate, &channels) < 1) {
-		LOGP(DLMGCP, LOGL_ERROR, "Invalid audio codec: %s\n", osmo_quote_str(audio_name, -1));
+	if (sscanf(codec->audio_name, "%63[^/]/%d/%d", codec->subtype_name, &rate, &channels) < 1) {
+		LOGP(DLMGCP, LOGL_ERROR, "Invalid audio codec: %s\n", osmo_quote_str(codec->audio_name, -1));
 		goto error;
 	}
 
 	/* Note: We only accept configurations with one audio channel! */
 	if (channels != 1) {
 		LOGP(DLMGCP, LOGL_ERROR, "Cannot handle audio codec with more than one channel: %s\n",
-		     osmo_quote_str(audio_name, -1));
+		     osmo_quote_str(codec->audio_name, -1));
 		goto error;
 	}
 
 	codec->rate = rate;
 	codec->channels = channels;
-	codec->subtype_name = talloc_strdup(ctx, audio_codec);
-	codec->audio_name = talloc_strdup(ctx, audio_name);
 	codec->payload_type = payload_type;
 
-	if (!strcmp(audio_codec, "G729")) {
+	if (!strcmp(codec->subtype_name, "G729")) {
 		codec->frame_duration_num = 10;
 		codec->frame_duration_den = 1000;
 	} else {
@@ -287,7 +283,7 @@
 	/* A codec name must be set, if not, this might mean that the codec
 	 * (payload type) that was assigned is unknown to us so we must stop
 	 * here. */
-	if (!codec->subtype_name)
+	if (!strlen(codec->subtype_name))
 		return false;
 
 	/* FIXME: implement meaningful checks to make sure that the given codec
diff --git a/src/libosmo-mgcp/mgcp_sdp.c b/src/libosmo-mgcp/mgcp_sdp.c
index eabaf53..077ac96 100644
--- a/src/libosmo-mgcp/mgcp_sdp.c
+++ b/src/libosmo-mgcp/mgcp_sdp.c
@@ -439,7 +439,7 @@
 	for (i = 0; i < codecs_used; i++) {
 		LOGPC(DLMGCP, LOGL_NOTICE, "%d=%s",
 		      rtp->codecs[i].payload_type,
-		      rtp->codecs[i].subtype_name ? rtp-> codecs[i].subtype_name : "unknown");
+		      strlen(rtp->codecs[i].subtype_name) ? rtp->codecs[i].subtype_name : "unknown");
 		LOGPC(DLMGCP, LOGL_NOTICE, " ");
 	}
 	LOGPC(DLMGCP, LOGL_NOTICE, "\n");

-- 
To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/25421
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Change-Id: Icc41643050a5e1ca3c66f307d60b6911ba1b8032
Gerrit-Change-Number: 25421
Gerrit-PatchSet: 8
Gerrit-Owner: Hoernchen <ewild at sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge at osmocom.org>
Gerrit-Reviewer: pespin <pespin at sysmocom.de>
Gerrit-CC: dexter <pmaier at sysmocom.de>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20210913/3f46ecab/attachment.htm>


More information about the gerrit-log mailing list