Access union value_data members consistently

Use the same, appropriate union value_data member for each access of BOOLEAN, BYTE and SHORT PW_TYPEs, without assuming they're interchangeable with "integer", as that is only true on little-endian architectures. This fixes at least this wimax unit test failure on s390x and ppc64: Mismatch in line 11 of src/tests/unit/wimax.txt, got: 1a 0c 00 00 60 b5 01 06 00 02 03 00 expected: 1a 0c 00 00 60 b5 01 06 00 02 03 01
FreeRADIUS · Oct 1, 2014 · 6f7706f · 6f7706f
1 parent 430991f
commit 6f7706f
Show file tree

Hide file tree

Showing 7 changed files with 53 additions and 18 deletions.
diff --git a/src/lib/print.c b/src/lib/print.c
@@ -370,16 +370,24 @@ size_t vp_data_prints_value(char *out, size_t outlen,
 	case PW_TYPE_INTEGER:
 	case PW_TYPE_BYTE:
 	case PW_TYPE_SHORT:
+	{
+		unsigned int i = (type == PW_TYPE_INTEGER)
+					? data->integer
+					: ((type == PW_TYPE_SHORT)
+						? data->ushort
+						: data->byte);
+
 		/* Normal, non-tagged attribute */
-		if (values && (v = dict_valbyattr(values->attr, values->vendor, data->integer)) != NULL) {
+		if (values && (v = dict_valbyattr(values->attr, values->vendor, i)) != NULL) {
 			a = v->name;
 			len = strlen(a);
 		} else {
 			/* should never be truncated */
-			len = snprintf(buf, sizeof(buf), "%u", data->integer);
+			len = snprintf(buf, sizeof(buf), "%u", i);
 			a = buf;
 		}
 		break;
+	}
 
 	case PW_TYPE_INTEGER64:
 		return snprintf(out, outlen, "%" PRIu64, data->integer64);

diff --git a/src/lib/radius.c b/src/lib/radius.c
@@ -3984,18 +3984,18 @@ ssize_t rad_vp2data(uint8_t const **out, VALUE_PAIR const *vp)
 	}
 
 	case PW_TYPE_BOOLEAN:
-		buffer[0] = vp->vp_integer & 0x01;
+		buffer[0] = vp->vp_byte & 0x01;
 		*out = buffer;
 		break;
 
 	case PW_TYPE_BYTE:
-		buffer[0] = vp->vp_integer & 0xff;
+		buffer[0] = vp->vp_byte & 0xff;
 		*out = buffer;
 		break;
 
 	case PW_TYPE_SHORT:
-		buffer[0] = (vp->vp_integer >> 8) & 0xff;
-		buffer[1] = vp->vp_integer & 0xff;
+		buffer[0] = (vp->vp_short >> 8) & 0xff;
+		buffer[1] = vp->vp_short & 0xff;
 		*out = buffer;
 		break;
 

diff --git a/src/lib/valuepair.c b/src/lib/valuepair.c
@@ -1372,42 +1372,64 @@ int pairparsevalue(VALUE_PAIR *vp, char const *value, size_t inlen)
 	case PW_TYPE_BYTE:
 	{
 		char *p;
+		unsigned int i;
 		vp->length = 1;
 
 		/*
 		 *	Note that ALL integers are unsigned!
 		 */
-		vp->vp_integer = fr_strtoul(value, &p);
+		i = fr_strtoul(value, &p);
+		vp->vp_byte = i;
 		if (!*p) {
-			if (vp->vp_integer > 255) {
+			if (i > 255) {
 				fr_strerror_printf("Byte value \"%s\" is larger than 255", value);
 				return -1;
 			}
 			break;
 		}
 		if (is_whitespace(p)) break;
+		/*
+		 *	Look for the named value for the given
+		 *	attribute.
+		 */
+		if ((dval = dict_valbyname(vp->da->attr, vp->da->vendor, value)) == NULL) {
+			fr_strerror_printf("Unknown value '%s' for attribute '%s'", value, vp->da->name);
+			return -1;
+		}
+		vp->vp_byte = dval->value;
+		break;
 	}
-		goto check_for_value;
 
 	case PW_TYPE_SHORT:
 	{
 		char *p;
+		unsigned int i;
 
 		/*
 		 *	Note that ALL integers are unsigned!
 		 */
-		vp->vp_integer = fr_strtoul(value, &p);
+		i = fr_strtoul(value, &p);
+		vp->vp_short = i;
 		vp->length = 2;
 		if (!*p) {
-			if (vp->vp_integer > 65535) {
+			if (i > 65535) {
 				fr_strerror_printf("Byte value \"%s\" is larger than 65535", value);
 				return -1;
 			}
 			break;
 		}
 		if (is_whitespace(p)) break;
+		/*
+		 *	Look for the named value for the given
+		 *	attribute.
+		 */
+		if ((dval = dict_valbyname(vp->da->attr, vp->da->vendor, value)) == NULL) {
+			fr_strerror_printf("Unknown value '%s' for attribute '%s'", value, vp->da->name);
+			return -1;
+		}
+		vp->vp_short = dval->value;
+		break;
 	}
-		goto check_for_value;
 
 	case PW_TYPE_INTEGER:
 	{
@@ -1421,7 +1443,6 @@ int pairparsevalue(VALUE_PAIR *vp, char const *value, size_t inlen)
 		if (!*p) break;
 		if (is_whitespace(p)) break;
 
-	check_for_value:
 		/*
 		 *	Look for the named value for the given
 		 *	attribute.

diff --git a/src/main/evaluate.c b/src/main/evaluate.c
@@ -448,11 +448,11 @@ static int do_cast_copy(VALUE_PAIR *dst, VALUE_PAIR const *src)
 			break;
 
 		case PW_TYPE_SHORT:
-			dst->vp_integer = ntohs(*(uint16_t const *) src->vp_octets);
+			dst->vp_short = ntohs(*(uint16_t const *) src->vp_octets);
 			break;
 
 		case PW_TYPE_BYTE:
-			dst->vp_integer = src->vp_octets[0];
+			dst->vp_byte = src->vp_octets[0];
 			break;
 
 		default:

diff --git a/src/main/valuepair.c b/src/main/valuepair.c
@@ -188,7 +188,11 @@ int radius_compare_vps(UNUSED REQUEST *request, VALUE_PAIR *check, VALUE_PAIR *v
 			break;
 
 		case PW_TYPE_BYTE:
+			ret = vp->vp_byte - check->vp_byte;
+			break;
 		case PW_TYPE_SHORT:
+			ret = vp->vp_short - check->vp_short;
+			break;
 		case PW_TYPE_INTEGER:
 			ret = vp->vp_integer - check->vp_integer;
 			break;

diff --git a/src/main/xlat.c b/src/main/xlat.c
@@ -171,9 +171,11 @@ static ssize_t xlat_integer(UNUSED void *instance, REQUEST *request,
 
 	case PW_TYPE_INTEGER:
 	case PW_TYPE_DATE:
+		return snprintf(out, outlen, "%u", vp->vp_integer);
 	case PW_TYPE_BYTE:
+		return snprintf(out, outlen, "%hhu", vp->vp_byte);
 	case PW_TYPE_SHORT:
-		return snprintf(out, outlen, "%u", vp->vp_integer);
+		return snprintf(out, outlen, "%hu", vp->vp_short);
 
 	/*
 	 *	Ethernet is weird... It's network related, so we assume to it should be

diff --git a/src/modules/rlm_eap/types/rlm_eap_ttls/ttls.c b/src/modules/rlm_eap/types/rlm_eap_ttls/ttls.c
@@ -325,12 +325,12 @@ static VALUE_PAIR *diameter2vp(REQUEST *request, REQUEST *fake, SSL *ssl,
 
 		case PW_TYPE_BYTE:
 			if (size != vp->length) goto raw;
-			vp->vp_integer = data[0];
+			vp->vp_byte = data[0];
 			break;
 
 		case PW_TYPE_SHORT:
 			if (size != vp->length) goto raw;
-			vp->vp_integer = (data[0] * 256) + data[1];
+			vp->vp_short = (data[0] * 256) + data[1];
 			break;
 
 		case PW_TYPE_SIGNED: