[MERGED] osmo-mgw[master]: libosmo-mgcp-client: fix debian, make self-contained

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/.

Neels Hofmeyr gerrit-no-reply at lists.osmocom.org
Sun Sep 24 20:21:53 UTC 2017


Neels Hofmeyr has submitted this change and it was merged.

Change subject: libosmo-mgcp-client: fix debian, make self-contained
......................................................................


libosmo-mgcp-client: fix debian, make self-contained

Add mgcp_common.h to libosmo-mgcp-client, to not need to share a header file
with libosmo-legacy-mgcp (nor the upcoming libosmo-mgcp). Both libraries use
the enum mgcp_connection_mode (and a for-loop macro and a string mangling
function), so far declared in mgcp.h, and both mgcp-dev and mgcp-client-dev
debian packages would require this header file to be installed. So far the
mgcp-client-dev lacks this header file, which causes the osmo-msc debian
package to fail building. Ways to solve:
- If both -dev debian packages installed the same header file in the same
  place, they would conflict if ever installed at the same time.
- mgcp-client-dev can depend on mgcp-dev, but it seems bad to keep such a large
  dependency for just one enum and two helpers.
- Instead, this patch solves this by copying the few definitions to
  libosmo-mgcp-client.

Once libosmo-mgcp-client has its own copy of those definitions, it is
fully self-contained and depending builds (osmo-msc deb) will succeed.

Copy the few actually common definitions to new header
<osmocom/mgcp_client/mgcp_common.h>. The nature of this .h is that it may be
shared with future libosmo-mgcp without causing linking problems.

Remove libosmo-legacy-mgcp/mgcp_common.c file from the build of
libosmo-mgcp-client, no longer needed.

Add to new mgcp_common.h:
- enum mgcp_connection_mode;
- for_each_non_empty_line() macro.
- mgcp_msg_terminate_nul() as static function.  Its complexity is just above
  your average static inline, but being inline is a way to use it in both mgcp
  and mgcp_client without linking problems.

Replace for_each_line() use in mgcp_client with for_each_non_empty_line()
(for_each_non_empty_line() replaces for_each_line() and uses strtok_r() instead
of a local reinvention).

mgcp_connection_mode_strs are actually only used in libosmo-mgcp-client, so
rename to mgcp_client_ prefix and move to mgcp_client.c.

BTW, the future plan for upcoming libosmo-mgcp is to use the identical header
file, and keep only one copy in the git source tree. The second copy may be
generated during 'make', to avoid code dup while having two distinct headers.

Related: I8e3359bedf973077c0a038aa04f5371a00c48fa0 (fix osmo-msc after this),
         I7a5d3b9a2eb90be7e34b95efa529429f2e6c3ed8 (mgcp_common.h)
Change-Id: Ifb8f3fc2b399662a9dbba174e942352a1a21df3f
---
M include/Makefile.am
M include/osmocom/legacy_mgcp/mgcp.h
M include/osmocom/mgcp_client/mgcp_client.h
M include/osmocom/mgcp_client/mgcp_client_internal.h
A include/osmocom/mgcp_client/mgcp_common.h
M src/libosmo-mgcp-client/Makefile.am
M src/libosmo-mgcp-client/mgcp_client.c
M src/libosmo-mgcp-client/mgcp_client_vty.c
M tests/mgcp_client/mgcp_client_test.c
9 files changed, 94 insertions(+), 15 deletions(-)

Approvals:
  Neels Hofmeyr: Looks good to me, approved
  Jenkins Builder: Verified



diff --git a/include/Makefile.am b/include/Makefile.am
index 94d74bb..0027386 100644
--- a/include/Makefile.am
+++ b/include/Makefile.am
@@ -7,4 +7,5 @@
 	osmocom/legacy_mgcp/mgcp_internal.h \
 	osmocom/legacy_mgcp/osmux.h \
 	osmocom/mgcp_client/mgcp_client.h \
+	osmocom/mgcp_client/mgcp_common.h \
 	$(NULL)
diff --git a/include/osmocom/legacy_mgcp/mgcp.h b/include/osmocom/legacy_mgcp/mgcp.h
index c017faf..147a0d5 100644
--- a/include/osmocom/legacy_mgcp/mgcp.h
+++ b/include/osmocom/legacy_mgcp/mgcp.h
@@ -177,13 +177,6 @@
 	MGCP_CONN_LOOPBACK  = 4 | MGCP_CONN_RECV_SEND,
 };
 
-extern const struct value_string mgcp_connection_mode_strs[];
-
-static inline const char *mgcp_cmode_name(enum mgcp_connection_mode mode)
-{
-	return get_value_string(mgcp_connection_mode_strs, mode);
-}
-
 struct mgcp_config {
 	int source_port;
 	char *local_ip;
diff --git a/include/osmocom/mgcp_client/mgcp_client.h b/include/osmocom/mgcp_client/mgcp_client.h
index df73811..efc1f76 100644
--- a/include/osmocom/mgcp_client/mgcp_client.h
+++ b/include/osmocom/mgcp_client/mgcp_client.h
@@ -2,6 +2,8 @@
 
 #include <stdint.h>
 
+#include <osmocom/mgcp_client/mgcp_common.h>
+
 #define MGCP_CLIENT_LOCAL_ADDR_DEFAULT "0.0.0.0"
 #define MGCP_CLIENT_LOCAL_PORT_DEFAULT 0
 #define MGCP_CLIENT_REMOTE_ADDR_DEFAULT "127.0.0.1"
@@ -71,3 +73,9 @@
 
 struct msgb *mgcp_msg_dlcx(struct mgcp_client *mgcp, uint16_t rtp_endpoint,
 			   unsigned int call_id);
+
+extern const struct value_string mgcp_client_connection_mode_strs[];
+static inline const char *mgcp_client_cmode_name(enum mgcp_connection_mode mode)
+{
+	return get_value_string(mgcp_client_connection_mode_strs, mode);
+}
diff --git a/include/osmocom/mgcp_client/mgcp_client_internal.h b/include/osmocom/mgcp_client/mgcp_client_internal.h
index 1b149e2..690a4af 100644
--- a/include/osmocom/mgcp_client/mgcp_client_internal.h
+++ b/include/osmocom/mgcp_client/mgcp_client_internal.h
@@ -1,5 +1,7 @@
 #pragma once
 
+#include <osmocom/core/write_queue.h>
+
 #define MSGB_CB_MGCP_TRANS_ID 0
 
 struct mgcp_client {
diff --git a/include/osmocom/mgcp_client/mgcp_common.h b/include/osmocom/mgcp_client/mgcp_common.h
new file mode 100644
index 0000000..0eb1388
--- /dev/null
+++ b/include/osmocom/mgcp_client/mgcp_common.h
@@ -0,0 +1,71 @@
+/* MGCP common implementations.
+ * These are used in libosmo-mgcp as well as libosmo-mgcp-client.
+ * To avoid interdependency, these are implemented in .h file only. */
+
+/*
+ * (C) 2017 by sysmocom s.f.m.c. GmbH <info at sysmocom.de>
+ * (C) 2009-2012 by Holger Hans Peter Freyther <zecke at selfish.org>
+ * (C) 2009-2012 by On-Waves
+ * All Rights Reserved
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as published by
+ * the Free Software Foundation; either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ */
+
+/* Two copies of this file are kept in osmocom/mgcp/ and osmocom/mgcp_client/.
+ * Since both are by definition identical, use the old header exclusion ifdefs
+ * instead of '#pragma once' to avoid including both of these files.
+ * Though at the time of writing there are no such users, this allows including
+ * both libosmo-mgcp and libosmo-mgcp-client headers in the same file. */
+#ifndef OSMO_MGCP_COMMON_H
+#define OSMO_MGCP_COMMON_H
+
+#include <string.h>
+#include <errno.h>
+
+#include <osmocom/core/msgb.h>
+#include <osmocom/core/logging.h>
+
+#define for_each_non_empty_line(line, save)			\
+	for (line = strtok_r(NULL, "\r\n", &save); line;	\
+	     line = strtok_r(NULL, "\r\n", &save))
+
+enum mgcp_connection_mode {
+	MGCP_CONN_NONE = 0,
+	MGCP_CONN_RECV_ONLY = 1,
+	MGCP_CONN_SEND_ONLY = 2,
+	MGCP_CONN_RECV_SEND = MGCP_CONN_RECV_ONLY | MGCP_CONN_SEND_ONLY,
+	MGCP_CONN_LOOPBACK  = 4 | MGCP_CONN_RECV_SEND,
+};
+
+/* Ensure that the msg->l2h is NUL terminated. */
+static inline int mgcp_msg_terminate_nul(struct msgb *msg)
+{
+	unsigned char *tail = msg->l2h + msgb_l2len(msg); /* char after l2 data */
+	if (tail[-1] == '\0')
+		/* nothing to do */;
+	else if (msgb_tailroom(msg) > 0)
+		tail[0] = '\0';
+	else if (tail[-1] == '\r' || tail[-1] == '\n')
+		tail[-1] = '\0';
+	else {
+		LOGP(DLMGCP, LOGL_ERROR, "Cannot NUL terminate MGCP message: "
+		     "Length: %d, Buffer size: %d\n",
+		     msgb_l2len(msg), msg->data_len);
+		return -ENOTSUP;
+	}
+	return 0;
+}
+
+#endif
diff --git a/src/libosmo-mgcp-client/Makefile.am b/src/libosmo-mgcp-client/Makefile.am
index b17477a..a2eb2be 100644
--- a/src/libosmo-mgcp-client/Makefile.am
+++ b/src/libosmo-mgcp-client/Makefile.am
@@ -30,7 +30,6 @@
 libosmo_mgcp_client_la_SOURCES = \
 	mgcp_client.c \
 	mgcp_client_vty.c \
-	../libosmo-legacy-mgcp/mgcp_common.c \
 	$(NULL)
 
 libosmo_mgcp_client_la_LDFLAGS = $(AM_LDFLAGS) -version-info $(MGCP_CLIENT_LIBVERSION)
diff --git a/src/libosmo-mgcp-client/mgcp_client.c b/src/libosmo-mgcp-client/mgcp_client.c
index b72fc50..1cd37be 100644
--- a/src/libosmo-mgcp-client/mgcp_client.c
+++ b/src/libosmo-mgcp-client/mgcp_client.c
@@ -24,8 +24,6 @@
 #include <osmocom/core/msgb.h>
 #include <osmocom/core/logging.h>
 
-#include <osmocom/legacy_mgcp/mgcp.h>
-#include <osmocom/legacy_mgcp/mgcp_internal.h>
 #include <osmocom/mgcp_client/mgcp_client.h>
 #include <osmocom/mgcp_client/mgcp_client_internal.h>
 
@@ -208,7 +206,7 @@
 	*data = '\0';
 	data ++;
 
-	for_each_line(line, data) {
+	for_each_non_empty_line(line, data) {
 		if (!mgcp_line_is_valid(line))
 			return -EINVAL;
 
@@ -584,7 +582,7 @@
 		 trans_id,
 		 rtp_endpoint,
 		 call_id,
-		 mgcp_cmode_name(mode));
+		 mgcp_client_cmode_name(mode));
 }
 
 struct msgb *mgcp_msg_mdcx(struct mgcp_client *mgcp,
@@ -602,7 +600,7 @@
 		 ,
 		 trans_id,
 		 rtp_endpoint,
-		 mgcp_cmode_name(mode),
+		 mgcp_client_cmode_name(mode),
 		 rtp_conn_addr,
 		 rtp_port);
 }
@@ -620,3 +618,12 @@
 {
 	return &mgcp->actual;
 }
+
+const struct value_string mgcp_client_connection_mode_strs[] = {
+	{ MGCP_CONN_NONE, "none" },
+	{ MGCP_CONN_RECV_SEND, "sendrecv" },
+	{ MGCP_CONN_SEND_ONLY, "sendonly" },
+	{ MGCP_CONN_RECV_ONLY, "recvonly" },
+	{ MGCP_CONN_LOOPBACK, "loopback" },
+	{ 0, NULL }
+};
diff --git a/src/libosmo-mgcp-client/mgcp_client_vty.c b/src/libosmo-mgcp-client/mgcp_client_vty.c
index 1e8bba6..c52803f 100644
--- a/src/libosmo-mgcp-client/mgcp_client_vty.c
+++ b/src/libosmo-mgcp-client/mgcp_client_vty.c
@@ -27,7 +27,6 @@
 #include <osmocom/vty/command.h>
 #include <osmocom/core/utils.h>
 
-#include <osmocom/legacy_mgcp/vty.h>
 #include <osmocom/mgcp_client/mgcp_client.h>
 
 #define MGCPGW_STR "MGCP gateway configuration for RTP streams\n"
diff --git a/tests/mgcp_client/mgcp_client_test.c b/tests/mgcp_client/mgcp_client_test.c
index 6045297..f2f0e0f 100644
--- a/tests/mgcp_client/mgcp_client_test.c
+++ b/tests/mgcp_client/mgcp_client_test.c
@@ -22,7 +22,6 @@
 
 #include <osmocom/core/msgb.h>
 #include <osmocom/core/application.h>
-#include <osmocom/legacy_mgcp/mgcp.h>
 #include <osmocom/mgcp_client/mgcp_client.h>
 #include <osmocom/mgcp_client/mgcp_client_internal.h>
 

-- 
To view, visit https://gerrit.osmocom.org/4010
To unsubscribe, visit https://gerrit.osmocom.org/settings

Gerrit-MessageType: merged
Gerrit-Change-Id: Ifb8f3fc2b399662a9dbba174e942352a1a21df3f
Gerrit-PatchSet: 3
Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Owner: Neels Hofmeyr <nhofmeyr at sysmocom.de>
Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: Max <msuraev at sysmocom.de>
Gerrit-Reviewer: Neels Hofmeyr <nhofmeyr at sysmocom.de>



More information about the gerrit-log mailing list