Change in osmo-mgw[master]: mgcp_verify_ci(): return meaningful error codes

[Neels Hofmeyr]

Neels Hofmeyr has submitted this change and it was merged. ( https://gerrit.osmocom.org/10753 )

Change subject: mgcp_verify_ci(): return meaningful error codes
......................................................................

mgcp_verify_ci(): return meaningful error codes

Instead of just -1, return RFC3435 error codes that can be used to compose a
FAIL message response. Note that the return value stays compatible in that it
returns 0 on a valid Connection Identifier, nonzero otherwise.

The idea is to be able to distinguish between "Conn ID not found" and "Conn ID
invalid" in mgcp_test.c's expected output, in upcoming change
I8d6cc96be252bb486e94f343a8c7cae641ff9429.

Change-Id: Ifc17f2893cc4b9a865f3ffcb9888bbf1039337a6
---
M src/libosmo-mgcp/mgcp_msg.c
M src/libosmo-mgcp/mgcp_protocol.c
2 files changed, 12 insertions(+), 11 deletions(-)

Approvals:
  Jenkins Builder: Verified
  Harald Welte: Looks good to me, approved



diff --git a/src/libosmo-mgcp/mgcp_msg.c b/src/libosmo-mgcp/mgcp_msg.c
index c4e66ff..f732158 100644
--- a/src/libosmo-mgcp/mgcp_msg.c
+++ b/src/libosmo-mgcp/mgcp_msg.c
@@ -430,16 +430,19 @@
 /*! Check if the specified connection id seems plausible.
   * \param[in] endp pointer to endpoint
   * \param{in] connection id to verify
-  * \returns 0 when connection id is valid and exists, nozero on error.
+  * \returns 0 when connection id is valid and exists, an RFC3435 error code on error.
   */
 int mgcp_verify_ci(struct mgcp_endpoint *endp, const char *conn_id)
 {
+	/* For invalid conn_ids, return 510 "The transaction could not be executed, because some
+	 * unspecified protocol error was detected." */
+
 	/* Check for null identifiers */
 	if (!conn_id) {
 		LOGP(DLMGCP, LOGL_ERROR,
 		     "endpoint:0x%x invalid ConnectionIdentifier (missing)\n",
 		     ENDPOINT_NUMBER(endp));
-		return -1;
+		return 510;
 	}
 
 	/* Check for empty connection identifiers */
@@ -447,7 +450,7 @@
 		LOGP(DLMGCP, LOGL_ERROR,
 		     "endpoint:0x%x invalid ConnectionIdentifier (empty)\n",
 		     ENDPOINT_NUMBER(endp));
-		return -1;
+		return 510;
 	}
 
 	/* Check for over long connection identifiers */
@@ -455,7 +458,7 @@
 		LOGP(DLMGCP, LOGL_ERROR,
 		     "endpoint:0x%x invalid ConnectionIdentifier (too long) 0x%s\n",
 		     ENDPOINT_NUMBER(endp), conn_id);
-		return -1;
+		return 510;
 	}
 
 	/* Check if connection exists */
@@ -466,7 +469,9 @@
 	     "endpoint:0x%x no connection found under ConnectionIdentifier 0x%s\n",
 	     ENDPOINT_NUMBER(endp), conn_id);
 
-	return -1;
+	/* When the conn_id was not found, return error code 515 "The transaction refers to an incorrect
+	 * connection-id (may have been already deleted)." */
+	return 515;
 }
 
 /*! Extract individual lines from MCGP message.
diff --git a/src/libosmo-mgcp/mgcp_protocol.c b/src/libosmo-mgcp/mgcp_protocol.c
index 7f4a7b8..bc191b5 100644
--- a/src/libosmo-mgcp/mgcp_protocol.c
+++ b/src/libosmo-mgcp/mgcp_protocol.c
@@ -999,10 +999,8 @@
 			break;
 		case 'I':
 			conn_id = (const char *)line + 3;
-			if (mgcp_verify_ci(endp, conn_id) != 0) {
-				error_code = 515;
+			if ((error_code = mgcp_verify_ci(endp, conn_id)))
 				goto error3;
-			}
 			break;
 		case 'L':
 			local_options = (const char *)line + 3;
@@ -1185,10 +1183,8 @@
 			break;
 		case 'I':
 			conn_id = (const char *)line + 3;
-			if (mgcp_verify_ci(endp, conn_id) != 0) {
-				error_code = 515;
+			if ((error_code = mgcp_verify_ci(endp, conn_id)))
 				goto error3;
-			}
 			break;
 		case 'Z':
 			silent = strcmp("noanswer", line + 3) == 0;

-- 
To view, visit https://gerrit.osmocom.org/10753
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ifc17f2893cc4b9a865f3ffcb9888bbf1039337a6
Gerrit-Change-Number: 10753
Gerrit-PatchSet: 1
Gerrit-Owner: Neels Hofmeyr <nhofmeyr at sysmocom.de>
Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org>
Gerrit-Reviewer: Jenkins Builder (1000002)
Gerrit-Reviewer: Neels Hofmeyr <nhofmeyr at sysmocom.de>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20180906/edaba5f0/attachment.htm>