Change in osmo-sgsn[master]: use enums consistently instead of falling back to int

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

Stefan Sperling gerrit-no-reply at lists.osmocom.org
Thu Nov 15 15:21:39 UTC 2018


Stefan Sperling has submitted this change and it was merged. ( https://gerrit.osmocom.org/11656 )

Change subject: use enums consistently instead of falling back to int
......................................................................

use enums consistently instead of falling back to int

The two existing enums defined in gprs_sndcp_xid.h, for protocol
and data compression algorithm numbers respectively, were assigned
to 'int' variables when their values were copied to other structures.

This prevented the compiler from checking the enum value coverage
during switch statements and also tripped up Coverity scans looking
for enum value mismatch problems.

So instead of copying enums to ints, make use of the enums throughout.
Structures which can contain values from both enums now use a union
of both, forcing us to be very explicit about which set of values
we are dealing with.

Change-Id: I3771a5c59f4e6fee24083b3c914965baf192cbd7
Depends: If6f3598cd6da4643ff2214e21c0d21f6eff0eb67
Depends: I8444c1ed052707c76a979fb06cb018ac678defa7
Related: CID#149102
---
M include/osmocom/sgsn/gprs_sndcp_comp.h
M include/osmocom/sgsn/gprs_sndcp_xid.h
M src/gprs/gprs_sndcp.c
M src/gprs/gprs_sndcp_comp.c
M src/gprs/gprs_sndcp_dcomp.c
M src/gprs/gprs_sndcp_pcomp.c
M src/gprs/gprs_sndcp_xid.c
M tests/sndcp_xid/sndcp_xid_test.c
8 files changed, 180 insertions(+), 88 deletions(-)

Approvals:
  Jenkins Builder: Verified
  Pau Espin Pedrol: Looks good to me, approved



diff --git a/include/osmocom/sgsn/gprs_sndcp_comp.h b/include/osmocom/sgsn/gprs_sndcp_comp.h
index c04f7d4..e01a9d7 100644
--- a/include/osmocom/sgsn/gprs_sndcp_comp.h
+++ b/include/osmocom/sgsn/gprs_sndcp_comp.h
@@ -41,9 +41,9 @@
 	uint8_t comp[MAX_COMP];	/* see also: 6.5.1.1.5 and 6.6.1.1.5 */
 
 	/* Algorithm parameters */
-	int algo;		/* Algorithm type (see gprs_sndcp_xid.h) */
-	int compclass;		/* See gprs_sndcp_xid.h/c */
-	void *state;		/* Algorithm status and parameters */
+	union gprs_sndcp_comp_algo algo;
+	enum gprs_sndcp_xid_param_types compclass;	/* See gprs_sndcp_xid.h/c */
+	void *state;					/* Algorithm status and parameters */
 };
 
 #define MAX_COMP 16	/* Maximum number of possible pcomp/dcomp values */
diff --git a/include/osmocom/sgsn/gprs_sndcp_xid.h b/include/osmocom/sgsn/gprs_sndcp_xid.h
index e64bc52..0dce43e 100644
--- a/include/osmocom/sgsn/gprs_sndcp_xid.h
+++ b/include/osmocom/sgsn/gprs_sndcp_xid.h
@@ -32,6 +32,24 @@
 #define MAX_NSAPI 11	/* Maximum number usable NSAPIs */
 #define MAX_ROHC 16	/* Maximum number of ROHC compression profiles */
 
+/* According to: 3GPP TS 44.065, 6.5.1.1.4 Algorithm identifier */
+enum gprs_sndcp_hdr_comp_algo {
+	RFC_1144,		/* TCP/IP header compression, see also 6.5.2 */
+	RFC_2507,		/* TCP/UDP/IP header compression, see also: 6.5.3 */
+	ROHC			/* Robust Header Compression, see also 6.5.4 */
+};
+
+/* According to: 3GPP TS 44.065, 6.5.1.1.4 Algorithm identifier */
+enum gprs_sndcp_data_comp_algo {
+	V42BIS,			/* V.42bis data compression, see also 6.6.2 */
+	V44			/* V44 data compression, see also: 6.6.3 */
+};
+
+union gprs_sndcp_comp_algo {
+	enum gprs_sndcp_hdr_comp_algo pcomp;
+	enum gprs_sndcp_data_comp_algo dcomp;
+};
+
 /* According to: 3GPP TS 44.065, 6.5.1.1 Format of the protocol control
  * information compression field (Figure 7) and 3GPP TS 44.065, 
  * 6.6.1.1 Format of the data compression field (Figure 9) */
@@ -45,7 +63,7 @@
 	unsigned int entity;
 
 	/* Algorithm identifier, see also: 6.5.1.1.4 and 6.6.1.1.4 */
-	int algo;
+	union gprs_sndcp_comp_algo algo;
 
 	/* Number of contained PCOMP / DCOMP values */
 	uint8_t comp_len;
@@ -62,24 +80,12 @@
 	struct gprs_sndcp_dcomp_v44_params *v44_params;
 };
 
-/* According to: 3GPP TS 44.065, 6.5.1.1.4 Algorithm identifier */
-enum gprs_sndcp_hdr_comp_algo {
-	RFC_1144,		/* TCP/IP header compression, see also 6.5.2 */
-	RFC_2507,		/* TCP/UDP/IP header compression, see also: 6.5.3 */
-	ROHC			/* Robust Header Compression, see also 6.5.4 */
-};
-
-/* According to: 3GPP TS 44.065, 6.5.1.1.4 Algorithm identifier */
-enum gprs_sndcp_data_comp_algo {
-	V42BIS,			/* V.42bis data compression, see also 6.6.2 */
-	V44			/* V44 data compression, see also: 6.6.3 */
-};
-
 /* According to: 3GPP TS 44.065, 8 SNDCP XID parameters */
 enum gprs_sndcp_xid_param_types {
 	SNDCP_XID_VERSION_NUMBER,
 	SNDCP_XID_DATA_COMPRESSION,	/* See also: subclause 6.6.1 */
 	SNDCP_XID_PROTOCOL_COMPRESSION,	/* See also: subclause 6.5.1 */
+	SNDCP_XID_INVALID_COMPRESSION   /* Not part of the spec; this means we found an invalid value */
 };
 
 /* According to: 3GPP TS 44.065, 6.5.2.1 Parameters (Table 5) */
@@ -209,7 +215,7 @@
 
 /* Find out to which compression class the specified comp-field belongs
  * (header compression or data compression?) */
-int gprs_sndcp_get_compression_class(
+enum gprs_sndcp_xid_param_types gprs_sndcp_get_compression_class(
 				const struct gprs_sndcp_comp_field *comp_field);
 
 /* Dump a list with SNDCP-XID fields (Debug) */
diff --git a/src/gprs/gprs_sndcp.c b/src/gprs/gprs_sndcp.c
index 52eeb75..f0239cb 100644
--- a/src/gprs/gprs_sndcp.c
+++ b/src/gprs/gprs_sndcp.c
@@ -934,7 +934,7 @@
 		rfc1144_params.s01 = sgsn->cfg.pcomp_rfc1144.s01;
 		rfc1144_comp_field.p = 1;
 		rfc1144_comp_field.entity = entity;
-		rfc1144_comp_field.algo = RFC_1144;
+		rfc1144_comp_field.algo.pcomp = RFC_1144;
 		rfc1144_comp_field.comp[RFC1144_PCOMP1] = 1;
 		rfc1144_comp_field.comp[RFC1144_PCOMP2] = 2;
 		rfc1144_comp_field.comp_len = RFC1144_PCOMP_NUM;
@@ -952,7 +952,7 @@
 		v42bis_params.p2 = sgsn->cfg.dcomp_v42bis.p2;
 		v42bis_comp_field.p = 1;
 		v42bis_comp_field.entity = entity;
-		v42bis_comp_field.algo = V42BIS;
+		v42bis_comp_field.algo.dcomp = V42BIS;
 		v42bis_comp_field.comp[V42BIS_DCOMP1] = 1;
 		v42bis_comp_field.comp_len = V42BIS_DCOMP_NUM;
 		v42bis_comp_field.v42bis_params = &v42bis_params;
@@ -1021,7 +1021,7 @@
 	comp_field->p = 0;
 
 	/* Process proposed parameters */
-	switch (comp_field->algo) {
+	switch (comp_field->algo.pcomp) {
 	case RFC_1144:
 		if (sgsn->cfg.pcomp_rfc1144.passive
 		    && comp_field->rfc1144_params->nsapi_len > 0) {
@@ -1068,7 +1068,7 @@
 	comp_field->p = 0;
 
 	/* Process proposed parameters */
-	switch (comp_field->algo) {
+	switch (comp_field->algo.dcomp) {
 	case V42BIS:
 		if (sgsn->cfg.dcomp_v42bis.passive &&
 		    comp_field->v42bis_params->nsapi_len > 0) {
diff --git a/src/gprs/gprs_sndcp_comp.c b/src/gprs/gprs_sndcp_comp.c
index 3e02603..0b4c67c 100644
--- a/src/gprs/gprs_sndcp_comp.c
+++ b/src/gprs/gprs_sndcp_comp.c
@@ -55,32 +55,36 @@
 		memcpy(comp_entity->nsapi,
 		       comp_field->rfc1144_params->nsapi,
 		       sizeof(comp_entity->nsapi));
+		comp_entity->algo.pcomp = comp_field->algo.pcomp;
 	} else if (comp_field->rfc2507_params) {
 		comp_entity->nsapi_len = comp_field->rfc2507_params->nsapi_len;
 		memcpy(comp_entity->nsapi,
 		       comp_field->rfc2507_params->nsapi,
 		       sizeof(comp_entity->nsapi));
+		comp_entity->algo.pcomp = comp_field->algo.pcomp;
 	} else if (comp_field->rohc_params) {
 		comp_entity->nsapi_len = comp_field->rohc_params->nsapi_len;
 		memcpy(comp_entity->nsapi, comp_field->rohc_params->nsapi,
 		       sizeof(comp_entity->nsapi));
+		comp_entity->algo.pcomp = comp_field->algo.pcomp;
 	} else if (comp_field->v42bis_params) {
 		comp_entity->nsapi_len = comp_field->v42bis_params->nsapi_len;
 		memcpy(comp_entity->nsapi,
 		       comp_field->v42bis_params->nsapi,
 		       sizeof(comp_entity->nsapi));
+		comp_entity->algo.dcomp = comp_field->algo.dcomp;
 	} else if (comp_field->v44_params) {
 		comp_entity->nsapi_len = comp_field->v44_params->nsapi_len;
 		memcpy(comp_entity->nsapi,
 		       comp_field->v44_params->nsapi,
 		       sizeof(comp_entity->nsapi));
+		comp_entity->algo.dcomp = comp_field->algo.dcomp;
 	} else {
 		/* The caller is expected to check carefully if the all
 		 * data fields required for compression entity creation
 		 * are present. Otherwise we blow an assertion here */
 		OSMO_ASSERT(false);
 	}
-	comp_entity->algo = comp_field->algo;
 
 	/* Check if an NSAPI is selected, if not, it does not make sense
 	 * to create the compression entity, since the caller should
@@ -93,17 +97,20 @@
 	comp_entity->compclass = gprs_sndcp_get_compression_class(comp_field);
 
 	/* Create an algorithm specific compression context */
-	if (comp_entity->compclass == SNDCP_XID_PROTOCOL_COMPRESSION) {
+	switch (comp_entity->compclass) {
+	case SNDCP_XID_PROTOCOL_COMPRESSION:
 		if (gprs_sndcp_pcomp_init(ctx, comp_entity, comp_field) != 0) {
 			talloc_free(comp_entity);
 			comp_entity = NULL;
 		}
-	} else if (comp_entity->compclass == SNDCP_XID_DATA_COMPRESSION) {
+		break;
+	case SNDCP_XID_DATA_COMPRESSION:
 		if (gprs_sndcp_dcomp_init(ctx, comp_entity, comp_field) != 0) {
 			talloc_free(comp_entity);
 			comp_entity = NULL;
 		}
-	} else {
+		break;
+	default:
 		/* comp_field is somehow invalid */
 		OSMO_ASSERT(false);
 	}
diff --git a/src/gprs/gprs_sndcp_dcomp.c b/src/gprs/gprs_sndcp_dcomp.c
index 04ff491..00e40a7 100644
--- a/src/gprs/gprs_sndcp_dcomp.c
+++ b/src/gprs/gprs_sndcp_dcomp.c
@@ -83,7 +83,7 @@
 	OSMO_ASSERT(comp_field);
 
 	if (comp_entity->compclass == SNDCP_XID_DATA_COMPRESSION
-	    && comp_entity->algo == V42BIS) {
+	    && comp_entity->algo.dcomp == V42BIS) {
 		OSMO_ASSERT(comp_field->v42bis_params);
 		comp_entity->state =
 		    v42bis_init(ctx, NULL, comp_field->v42bis_params->p0,
@@ -114,7 +114,7 @@
 	OSMO_ASSERT(comp_entity);
 
 	if (comp_entity->compclass == SNDCP_XID_DATA_COMPRESSION
-	    && comp_entity->algo == V42BIS) {
+	    && comp_entity->algo.dcomp == V42BIS) {
 		if (comp_entity->state) {
 			v42bis_free((v42bis_state_t *) comp_entity->state);
 			comp_entity->state = NULL;
@@ -293,7 +293,7 @@
 
 	/* Note: Currently V42BIS is the only compression method we
 	 * support, so the only allowed algorithm is V42BIS */
-	OSMO_ASSERT(comp_entity->algo == V42BIS);
+	OSMO_ASSERT(comp_entity->algo.dcomp == V42BIS);
 
 	/* Find pcomp_index */
 	pcomp_index = gprs_sndcp_comp_get_idx(comp_entity, pcomp);
@@ -339,7 +339,7 @@
 
 	/* Note: Currently V42BIS is the only compression method we
 	 * support, so the only allowed algorithm is V42BIS */
-	OSMO_ASSERT(comp_entity->algo == V42BIS);
+	OSMO_ASSERT(comp_entity->algo.dcomp == V42BIS);
 
 	/* Run compression algo */
 	rc = v42bis_compress_unitdata(&pcomp_index, data, len,
diff --git a/src/gprs/gprs_sndcp_pcomp.c b/src/gprs/gprs_sndcp_pcomp.c
index 2911b5e..5f7f22a 100644
--- a/src/gprs/gprs_sndcp_pcomp.c
+++ b/src/gprs/gprs_sndcp_pcomp.c
@@ -53,7 +53,7 @@
 	OSMO_ASSERT(comp_field);
 
 	if (comp_entity->compclass == SNDCP_XID_PROTOCOL_COMPRESSION
-	    && comp_entity->algo == RFC_1144) {
+	    && comp_entity->algo.pcomp == RFC_1144) {
 		OSMO_ASSERT(comp_field->rfc1144_params);
 		comp_entity->state =
 		    slhc_init(ctx, comp_field->rfc1144_params->s01 + 1,
@@ -79,7 +79,7 @@
 	OSMO_ASSERT(comp_entity);
 
 	if (comp_entity->compclass == SNDCP_XID_PROTOCOL_COMPRESSION
-	    && comp_entity->algo == RFC_1144) {
+	    && comp_entity->algo.pcomp == RFC_1144) {
 		if (comp_entity->state) {
 			slhc_free((struct slcompress *)comp_entity->state);
 			comp_entity->state = NULL;
@@ -214,7 +214,7 @@
 
 	/* Note: Currently RFC1144 is the only compression method we
 	 * support, so the only allowed algorithm is RFC1144 */
-	OSMO_ASSERT(comp_entity->algo == RFC_1144);
+	OSMO_ASSERT(comp_entity->algo.pcomp == RFC_1144);
 
 	/* Find pcomp_index */
 	pcomp_index = gprs_sndcp_comp_get_idx(comp_entity, pcomp);
@@ -263,7 +263,7 @@
 
 	/* Note: Currently RFC1144 is the only compression method we
 	 * support, so the only allowed algorithm is RFC1144 */
-	OSMO_ASSERT(comp_entity->algo == RFC_1144);
+	OSMO_ASSERT(comp_entity->algo.pcomp == RFC_1144);
 
 	/* Run compression algo */
 	rc = rfc1144_compress(&pcomp_index, data, len, comp_entity->state);
diff --git a/src/gprs/gprs_sndcp_xid.c b/src/gprs/gprs_sndcp_xid.c
index 09e21f5..a19f645 100644
--- a/src/gprs/gprs_sndcp_xid.c
+++ b/src/gprs/gprs_sndcp_xid.c
@@ -42,9 +42,8 @@
  * about the referenced algorithm to the parser. */
 struct entity_algo_table {
 	unsigned int entity;	/* see also: 6.5.1.1.3 and 6.6.1.1.3 */
-	unsigned int algo;	/* see also: 6.5.1.1.4 and 6.6.1.1.4 */
-	unsigned int compclass;	/* Can be either SNDCP_XID_DATA_COMPRESSION or
-				   SNDCP_XID_PROTOCOL_COMPRESSION */
+	union gprs_sndcp_comp_algo algo;
+	enum gprs_sndcp_xid_param_types compclass;
 };
 
 /* FUNCTIONS RELATED TO SNDCP-XID ENCODING */
@@ -386,6 +385,7 @@
 	int len;
 	int expected_length;
 	int i;
+	enum gprs_sndcp_xid_param_types compclass;
 
 	uint8_t payload_bytes[256];
 	int payload_bytes_len = -1;
@@ -443,7 +443,17 @@
 	OSMO_ASSERT(comp_field->entity <= 0x1f);
 
 	/* Check if the algorithm number is within bounds */
-	OSMO_ASSERT(comp_field->algo >= 0 && comp_field->algo <= 0x1f);
+	compclass = gprs_sndcp_get_compression_class(comp_field);
+	switch (compclass) {
+	case SNDCP_XID_PROTOCOL_COMPRESSION:
+		OSMO_ASSERT(comp_field->algo.pcomp >= 0 && comp_field->algo.pcomp <= 0x1f);
+		break;
+	case SNDCP_XID_DATA_COMPRESSION:
+		OSMO_ASSERT(comp_field->algo.dcomp >= 0 && comp_field->algo.dcomp <= 0x1f);
+		break;
+	default:
+		OSMO_ASSERT(false);
+	}
 
 	/* Zero out buffer */
 	memset(dst, 0, dst_maxlen);
@@ -459,7 +469,10 @@
 
 	/* Encode algorithm number */
 	if (comp_field->p) {
-		*dst |= comp_field->algo & 0x1F;
+		if (compclass == SNDCP_XID_PROTOCOL_COMPRESSION)
+			*dst |= comp_field->algo.pcomp & 0x1F;
+		else
+			*dst |= comp_field->algo.dcomp & 0x1F;
 		dst++;
 		dst_counter++;
 	}
@@ -501,7 +514,7 @@
 
 /* Find out to which compression class the specified comp-field belongs
  * (header compression or data compression?) */
-int gprs_sndcp_get_compression_class(const struct gprs_sndcp_comp_field
+enum gprs_sndcp_xid_param_types gprs_sndcp_get_compression_class(const struct gprs_sndcp_comp_field
 				     *comp_field)
 {
 	OSMO_ASSERT(comp_field);
@@ -517,7 +530,7 @@
 	else if (comp_field->v44_params)
 		return SNDCP_XID_DATA_COMPRESSION;
 	else
-		return -EINVAL;
+		return SNDCP_XID_INVALID_COMPRESSION;
 }
 
 /* Convert all compression fields to bytstreams */
@@ -1005,10 +1018,10 @@
 	return byte_counter;
 }
 
-/* Lookup algorithm identfier by entity ID */
-static int lookup_algorithm_identifier(int entity, const struct
-				       entity_algo_table
-				       *lt, unsigned int lt_len, int compclass)
+/* Lookup protocol compression algorithm identfier by entity ID */
+static enum gprs_sndcp_hdr_comp_algo lookup_algorithm_identifier_pcomp(int entity,
+				       const struct entity_algo_table *lt,
+				       unsigned int lt_len)
 {
 	int i;
 
@@ -1017,26 +1030,46 @@
 
 	for (i = 0; i < lt_len; i++) {
 		if ((lt[i].entity == entity)
-		    && (lt[i].compclass == compclass))
-			return lt[i].algo;
+		    && (lt[i].compclass == SNDCP_XID_PROTOCOL_COMPRESSION))
+			return lt[i].algo.pcomp;
 	}
 
-	return -1;
+	return SNDCP_XID_INVALID_COMPRESSION;
+}
+
+/* Lookup a data compression algorithm identfier by entity ID */
+static enum gprs_sndcp_data_comp_algo lookup_algorithm_identifier_dcomp(int entity,
+				       const struct entity_algo_table *lt,
+				       unsigned int lt_len)
+{
+	int i;
+
+	if (!lt)
+		return -1;
+
+	for (i = 0; i < lt_len; i++) {
+		if ((lt[i].entity == entity)
+		    && (lt[i].compclass == SNDCP_XID_DATA_COMPRESSION))
+			return lt[i].algo.dcomp;
+	}
+
+	return SNDCP_XID_INVALID_COMPRESSION;
 }
 
 /* Helper function for decode_comp_field(), decodes
  * numeric pcomp/dcomp values */
 static int decode_comp_values(struct gprs_sndcp_comp_field *comp_field,
-			      const uint8_t *src, int compclass)
+			      const uint8_t *src, enum gprs_sndcp_xid_param_types compclass)
 {
 	int src_counter = 0;
 	int i;
 
 	if (comp_field->p) {
 		/* Determine the number of expected PCOMP/DCOMP values */
-		if (compclass == SNDCP_XID_PROTOCOL_COMPRESSION) {
+		switch (compclass) {
+		case SNDCP_XID_PROTOCOL_COMPRESSION:
 			/* For protocol compression */
-			switch (comp_field->algo) {
+			switch (comp_field->algo.pcomp) {
 			case RFC_1144:
 				comp_field->comp_len = RFC1144_PCOMP_NUM;
 				break;
@@ -1052,9 +1085,10 @@
 			default:
 				return -EINVAL;
 			}
-		} else {
+			break;
+		case SNDCP_XID_DATA_COMPRESSION:
 			/* For data compression */
-			switch (comp_field->algo) {
+			switch (comp_field->algo.dcomp) {
 			case V42BIS:
 				comp_field->comp_len = V42BIS_DCOMP_NUM;
 				break;
@@ -1067,6 +1101,9 @@
 			default:
 				return -EINVAL;
 			}
+			break;
+		default:
+			return -EINVAL;
 		}
 
 		for (i = 0; i < comp_field->comp_len; i++) {
@@ -1094,7 +1131,7 @@
 {
 	int rc;
 
-	switch (comp_field->algo) {
+	switch (comp_field->algo.pcomp) {
 	case RFC_1144:
 		comp_field->rfc1144_params = talloc_zero(comp_field, struct
 					gprs_sndcp_pcomp_rfc1144_params);
@@ -1142,7 +1179,7 @@
 {
 	int rc;
 
-	switch (comp_field->algo) {
+	switch (comp_field->algo.dcomp) {
 	case V42BIS:
 		comp_field->v42bis_params = talloc_zero(comp_field, struct
 					gprs_sndcp_dcomp_v42bis_params);
@@ -1180,7 +1217,8 @@
 static int decode_comp_field(struct gprs_sndcp_comp_field *comp_field,
 			     const uint8_t *src, unsigned int src_len,
 			     const struct entity_algo_table *lt,
-			     unsigned int lt_len, int compclass)
+			     unsigned int lt_len,
+			     enum gprs_sndcp_xid_param_types compclass)
 {
 	int src_counter = 0;
 	unsigned int len;
@@ -1205,15 +1243,34 @@
 
 	/* Decode algorithm number (if present) */
 	if (comp_field->p) {
-		comp_field->algo = (*src) & 0x1F;
+		switch (compclass) {
+		case SNDCP_XID_PROTOCOL_COMPRESSION:
+			comp_field->algo.pcomp = (*src) & 0x1F;
+			break;
+		case SNDCP_XID_DATA_COMPRESSION:
+			comp_field->algo.dcomp = (*src) & 0x1F;
+			break;
+		default:
+			return -EINVAL;
+		}
 		src_counter++;
 		src++;
 	}
 	/* Alternatively take the information from the lookup table */
-	else
-		comp_field->algo =
-		    lookup_algorithm_identifier(comp_field->entity, lt,
-						lt_len, compclass);
+	else {
+		switch (compclass) {
+		case SNDCP_XID_PROTOCOL_COMPRESSION:
+			comp_field->algo.pcomp =
+			    lookup_algorithm_identifier_pcomp(comp_field->entity, lt, lt_len);
+			break;
+		case SNDCP_XID_DATA_COMPRESSION:
+			comp_field->algo.dcomp =
+			    lookup_algorithm_identifier_dcomp(comp_field->entity, lt, lt_len);
+			break;
+		default:
+			return -EINVAL;
+		}
+	}
 
 	/* Decode length field */
 	len = *src;
@@ -1368,7 +1425,7 @@
 {
 	struct gprs_sndcp_comp_field *comp_field;
 	int i = 0;
-	int rc;
+	enum gprs_sndcp_xid_param_types compclass;
 
 	if (!comp_fields)
 		return -EINVAL;
@@ -1378,19 +1435,23 @@
 	memset(lt, 0, sizeof(*lt));
 
 	llist_for_each_entry(comp_field, comp_fields, list) {
-		if (comp_field->algo >= 0) {
-			lt[i].entity = comp_field->entity;
-			lt[i].algo = comp_field->algo;
-			rc = gprs_sndcp_get_compression_class(comp_field);
-
-			if (rc < 0) {
-				memset(lt, 0, sizeof(*lt));
-				return -EINVAL;
-			}
-
-			lt[i].compclass = rc;
-			i++;
+		compclass = gprs_sndcp_get_compression_class(comp_field);
+		switch (compclass) {
+		case SNDCP_XID_PROTOCOL_COMPRESSION:
+			lt[i].algo.pcomp = comp_field->algo.pcomp;
+			break;
+		case SNDCP_XID_DATA_COMPRESSION:
+			lt[i].algo.dcomp = comp_field->algo.dcomp;
+			break;
+		case SNDCP_XID_INVALID_COMPRESSION:
+			continue;
+		default:
+			memset(lt, 0, sizeof(*lt));
+			return -EINVAL;
 		}
+		lt[i].compclass = compclass;
+		lt[i].entity = comp_field->entity;
+		i++;
 	}
 
 	return i;
@@ -1402,7 +1463,10 @@
 				      *comp_field_dst, const struct
 				      gprs_sndcp_comp_field *comp_field_src)
 {
-	if (comp_field_dst->algo < 0)
+	enum gprs_sndcp_xid_param_types compclass;
+
+	compclass = gprs_sndcp_get_compression_class(comp_field_dst);
+	if (compclass == SNDCP_XID_INVALID_COMPRESSION)
 		return -EINVAL;
 
 	if (comp_field_dst->rfc1144_params && comp_field_src->rfc1144_params) {
@@ -1626,7 +1690,7 @@
 {
 	int i;
 
-	switch (comp_field->algo) {
+	switch (comp_field->algo.pcomp) {
 	case RFC_1144:
 		if (comp_field->rfc1144_params == NULL) {
 			LOGP(DSNDCP, logl,
@@ -1726,7 +1790,7 @@
 {
 	int i;
 
-	switch (comp_field->algo) {
+	switch (comp_field->algo.dcomp) {
 	case V42BIS:
 		if (comp_field->v42bis_params == NULL) {
 			LOGP(DSNDCP, logl,
@@ -1791,15 +1855,26 @@
 {
 	struct gprs_sndcp_comp_field *comp_field;
 	int i;
-	int compclass;
+	enum gprs_sndcp_xid_param_types compclass;
 
 	OSMO_ASSERT(comp_fields);
 
 	llist_for_each_entry(comp_field, comp_fields, list) {
+		compclass = gprs_sndcp_get_compression_class(comp_field);
 		LOGP(DSNDCP, logl, "SNDCP-XID:\n");
 		LOGP(DSNDCP, logl, "struct gprs_sndcp_comp_field {\n");
 		LOGP(DSNDCP, logl, "   entity=%d;\n", comp_field->entity);
-		LOGP(DSNDCP, logl, "   algo=%d;\n", comp_field->algo);
+		switch (compclass) {
+		case SNDCP_XID_PROTOCOL_COMPRESSION:
+			LOGP(DSNDCP, logl, "   algo=%d;\n", comp_field->algo.pcomp);
+			break;
+		case SNDCP_XID_DATA_COMPRESSION:
+			LOGP(DSNDCP, logl, "   algo=%d;\n", comp_field->algo.dcomp);
+			break;
+		default:
+			LOGP(DSNDCP, logl, "   algo invalid!\n");
+			break;
+		}
 		LOGP(DSNDCP, logl, "   comp_len=%d;\n", comp_field->comp_len);
 		if (comp_field->comp_len == 0)
 			LOGP(DSNDCP, logl, "   comp[] = NULL;\n");
@@ -1808,12 +1883,16 @@
 			     comp_field->comp[i]);
 		}
 
-		compclass = gprs_sndcp_get_compression_class(comp_field);
-
-		if (compclass == SNDCP_XID_PROTOCOL_COMPRESSION) {
+		switch (compclass) {
+		case SNDCP_XID_PROTOCOL_COMPRESSION:
 			dump_pcomp_params(comp_field, logl);
-		} else if (compclass == SNDCP_XID_DATA_COMPRESSION) {
+			break;
+		case SNDCP_XID_DATA_COMPRESSION:
 			dump_dcomp_params(comp_field, logl);
+			break;
+		default:
+			LOGP(DSNDCP, logl, "   compression algorithm invalid!\n");
+			break;
 		}
 
 		LOGP(DSNDCP, logl, "}\n");
diff --git a/tests/sndcp_xid/sndcp_xid_test.c b/tests/sndcp_xid/sndcp_xid_test.c
index 5ed695c..56b97e3 100644
--- a/tests/sndcp_xid/sndcp_xid_test.c
+++ b/tests/sndcp_xid/sndcp_xid_test.c
@@ -106,7 +106,7 @@
 	/* Setup rfc1144 compression field */
 	rfc1144_comp_field.p = 1;
 	rfc1144_comp_field.entity = 0;
-	rfc1144_comp_field.algo = RFC_1144;
+	rfc1144_comp_field.algo.pcomp = RFC_1144;
 	rfc1144_comp_field.comp[RFC1144_PCOMP1] = 1;
 	rfc1144_comp_field.comp[RFC1144_PCOMP2] = 2;
 	rfc1144_comp_field.comp_len = RFC1144_PCOMP_NUM;
@@ -126,7 +126,7 @@
 	/* Setup rfc2507 compression field */
 	rfc2507_comp_field.p = 1;
 	rfc2507_comp_field.entity = 1;
-	rfc2507_comp_field.algo = RFC_2507;
+	rfc2507_comp_field.algo.pcomp = RFC_2507;
 	rfc2507_comp_field.comp[RFC2507_PCOMP1] = 3;
 	rfc2507_comp_field.comp[RFC2507_PCOMP2] = 4;
 	rfc2507_comp_field.comp[RFC2507_PCOMP3] = 5;
@@ -173,7 +173,7 @@
 	/* Setup ROHC compression field */
 	rohc_comp_field.p = 1;
 	rohc_comp_field.entity = 2;
-	rohc_comp_field.algo = ROHC;
+	rohc_comp_field.algo.pcomp = ROHC;
 	rohc_comp_field.comp[ROHC_PCOMP1] = 8;
 	rohc_comp_field.comp[ROHC_PCOMP2] = 9;
 	rohc_comp_field.comp_len = ROHC_PCOMP_NUM;
@@ -191,7 +191,7 @@
 	/* Setup v42bis compression field */
 	v42bis_comp_field.p = 1;
 	v42bis_comp_field.entity = 3;
-	v42bis_comp_field.algo = V42BIS;
+	v42bis_comp_field.algo.dcomp = V42BIS;
 	v42bis_comp_field.comp[V42BIS_DCOMP1] = 10;
 	v42bis_comp_field.comp_len = V42BIS_DCOMP_NUM;
 	v42bis_comp_field.v42bis_params = &v42bis_params;
@@ -211,7 +211,7 @@
 	/* Setup v44 compression field */
 	v44_comp_field.p = 1;
 	v44_comp_field.entity = 3;
-	v44_comp_field.algo = V44;
+	v44_comp_field.algo.dcomp = V44;
 	v44_comp_field.comp[V44_DCOMP1] = 10;
 	v44_comp_field.comp[V44_DCOMP2] = 11;
 	v44_comp_field.comp_len = V44_DCOMP_NUM;

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

Gerrit-Project: osmo-sgsn
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I3771a5c59f4e6fee24083b3c914965baf192cbd7
Gerrit-Change-Number: 11656
Gerrit-PatchSet: 4
Gerrit-Owner: Stefan Sperling <ssperling at sysmocom.de>
Gerrit-Reviewer: Harald Welte <laforge at gnumonks.org>
Gerrit-Reviewer: Jenkins Builder (1000002)
Gerrit-Reviewer: Neels Hofmeyr <nhofmeyr at sysmocom.de>
Gerrit-Reviewer: Pau Espin Pedrol <pespin at sysmocom.de>
Gerrit-Reviewer: Stefan Sperling <ssperling at sysmocom.de>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20181115/55e29c1f/attachment.htm>


More information about the gerrit-log mailing list