[PATCH] acpi: method: add helper to check for package contents

Colin King colin.king at canonical.com
Thu Jan 10 13:07:50 UTC 2013


From: Colin Ian King <colin.king at canonical.com>

Add method_package_elements_all_type() that checks to see if the
package contains elements that are all of a specified type. This
allows us to remove a lot of duplicated code.

Signed-off-by: Colin Ian King <colin.king at canonical.com>
---
 src/acpi/method/method.c | 237 ++++++++++++++++++++---------------------------
 1 file changed, 103 insertions(+), 134 deletions(-)

diff --git a/src/acpi/method/method.c b/src/acpi/method/method.c
index 21c89c0..5b1cfdf 100644
--- a/src/acpi/method/method.c
+++ b/src/acpi/method/method.c
@@ -746,6 +746,64 @@ static void method_test_polling_return(
 	}
 }
 
+/*
+ *  Common types that can be returned. This is not a complete
+ *  list but it does cover the types we expect to return from
+ *  an ACPI evaluation.
+ */
+static const char *method_type_name(const ACPI_OBJECT_TYPE type)
+{
+	switch (type) {
+	case ACPI_TYPE_INTEGER:
+		return "integer";
+	case ACPI_TYPE_STRING:
+		return "string";
+	case ACPI_TYPE_BUFFER:
+		return "buffer";
+	case ACPI_TYPE_PACKAGE:
+		return "package";
+	case ACPI_TYPE_BUFFER_FIELD:
+		return "buffer_field";
+	case ACPI_TYPE_LOCAL_REFERENCE:
+		return "reference";
+	default:
+		return "unknown";
+	}
+}
+
+/*
+ *  method_package_elements_all_type()
+ *	sanity check fields in a package that all have
+ *	the same type
+ */
+static int method_package_elements_all_type(
+	fwts_framework *fw,
+	const char *name,
+	const char *objname,
+	const ACPI_OBJECT *obj,
+	const ACPI_OBJECT_TYPE type)
+{
+	uint32_t i;
+	bool failed = false;
+	char tmp[128];
+
+	for (i = 0; i < obj->Package.Count; i++) {
+		if (obj->Package.Elements[i].Type != type) {
+			snprintf(tmp, sizeof(tmp), "Method%sElementType", objname);
+			fwts_failed(fw, LOG_LEVEL_MEDIUM, tmp,
+				"%s package element %" PRIu32 " was not the expected "
+				"type '%s', was instead type '%s'.",
+				name, i,
+				method_type_name(type),
+				method_type_name(obj->Package.Elements[i].Type));
+			fwts_tag_failed(fw, FWTS_TAG_ACPI_METHOD_RETURN);
+			failed = true;
+		}
+	}
+
+	return failed ? FWTS_ERROR: FWTS_OK;
+}
+
 /****************************************************************************/
 
 /*
@@ -1855,28 +1913,16 @@ static void method_test_EDL_return(
 	ACPI_OBJECT *obj,
 	void *private)
 {
-	uint32_t i;
-	bool failed = false;
-
 	FWTS_UNUSED(private);
 
 	if (method_check_type(fw, name, buf, ACPI_TYPE_PACKAGE) != FWTS_OK)
 		return;
 
-	/* All elements in the package must be references */
-	for (i = 0; i < obj->Package.Count; i++) {
-		if (obj->Package.Elements[i].Type != ACPI_TYPE_LOCAL_REFERENCE) {
-			fwts_failed(fw, LOG_LEVEL_MEDIUM,
-				"Method_EDLElementType",
-				"%s package element %" PRIu32 " was not a reference.",
-				name, i);
-			fwts_tag_failed(fw, FWTS_TAG_ACPI_METHOD_RETURN);
-			failed = true;
-		}
-	}
+	if (method_package_elements_all_type(fw, name, "_EDL",
+		obj, ACPI_TYPE_LOCAL_REFERENCE) != FWTS_OK)
+		return;
 
-	if (!failed)
-		method_passed_sane(fw, name, "package");
+	method_passed_sane(fw, name, "package");
 }
 
 static int method_test_EDL(fwts_framework *fw)
@@ -2103,28 +2149,18 @@ static void method_test_power_resources_return(
 	ACPI_OBJECT *obj,
 	void *private)
 {
-	uint32_t i;
-	bool failed = false;
+	char *objname = (char *)private;
 
 	FWTS_UNUSED(private);
 
 	if (method_check_type(fw, name, buf, ACPI_TYPE_PACKAGE) != FWTS_OK)
 		return;
 
-	/* All elements in the package must be references */
-	for (i = 0; i < obj->Package.Count; i++) {
-		if (obj->Package.Elements[i].Type != ACPI_TYPE_LOCAL_REFERENCE) {
-			fwts_failed(fw, LOG_LEVEL_MEDIUM,
-				"Method_PowerResourceElementType",
-				"%s package element %" PRIu32 " was not a reference.",
-				name, i);
-			fwts_tag_failed(fw, FWTS_TAG_ACPI_METHOD_RETURN);
-			failed = true;
-		}
-	}
+	if (method_package_elements_all_type(fw, name, objname,
+		obj, ACPI_TYPE_LOCAL_REFERENCE) != FWTS_OK)
+		return;
 
-	if (!failed)
-		method_passed_sane(fw, name, "package");
+	method_passed_sane(fw, name, "package");
 }
 
 #define method_test_POWER(name)						\
@@ -2422,12 +2458,8 @@ static void method_test_CSD_return(
 		uint32_t j;
 		bool elements_ok = true;
 
-		if (obj->Package.Elements[i].Type != ACPI_TYPE_PACKAGE) {
-			fwts_failed(fw, LOG_LEVEL_MEDIUM,
-				"Method_CSDElementType",
-				"%s package element %" PRIu32 " was not a package.",
-				name, i);
-			fwts_tag_failed(fw, FWTS_TAG_ACPI_METHOD_RETURN);
+		if (method_package_elements_all_type(fw, name, "_CSD",
+			obj, ACPI_TYPE_PACKAGE) != FWTS_OK) {
 			failed = true;
 			continue;	/* Skip processing sub-package */
 		}
@@ -2694,9 +2726,6 @@ static void method_test_PCT_return(
 	ACPI_OBJECT *obj,
 	void *private)
 {
-	uint32_t i;
-	bool failed = false;
-
 	FWTS_UNUSED(private);
 
 	if (method_check_type(fw, name, buf, ACPI_TYPE_PACKAGE) != FWTS_OK)
@@ -2706,23 +2735,11 @@ static void method_test_PCT_return(
 	if (method_package_count_min(fw, name, "_PCT", obj, 2) != FWTS_OK)
 		return;
 
-	for (i = 0; i < obj->Package.Count; i++) {
-		/*
-		 * Fairly shallow checks here, should probably check
-		 * for a register description in the buffer
-		 */
-		if (obj->Package.Elements[i].Type != ACPI_TYPE_BUFFER) {
-			fwts_failed(fw, LOG_LEVEL_MEDIUM,
-				"Method_PCTElementType",
-				"%s package element %" PRIu32
-				" was not a buffer.", name, i);
-			fwts_tag_failed(fw, FWTS_TAG_ACPI_METHOD_RETURN);
-			failed = true;
-			continue;	/* Skip processing sub-package */
-		}
-	}
-	if (!failed)
-		method_passed_sane(fw, name, "package");
+	if (method_package_elements_all_type(fw, name, "_PCT",
+		obj, ACPI_TYPE_BUFFER) != FWTS_OK)
+		return;
+
+	method_passed_sane(fw, name, "package");
 }
 
 static int method_test_PCT(fwts_framework *fw)
@@ -3762,8 +3779,7 @@ static void method_test_BST_return(
 	ACPI_OBJECT *obj,
 	void *private)
 {
-	uint32_t i;
-	int failed = 0;
+	bool failed = false;
 
 	FWTS_UNUSED(private);
 
@@ -3773,17 +3789,8 @@ static void method_test_BST_return(
 	if (method_package_count_equal(fw, name, "_BST", obj, 4) != FWTS_OK)
 		return;
 
-	for (i = 0; (i < 4) && (i < obj->Package.Count); i++) {
-		if (obj->Package.Elements[i].Type != ACPI_TYPE_INTEGER) {
-			fwts_failed(fw, LOG_LEVEL_MEDIUM,
-				"Method_BSTBadType",
-				"%s package element %" PRIu32
-				" is not of type DWORD Integer.",
-				name, i);
-			fwts_tag_failed(fw, FWTS_TAG_ACPI_METHOD_RETURN);
-			failed++;
-		}
-	}
+	if (method_package_elements_all_type(fw, name, "_BST", obj, ACPI_TYPE_INTEGER) != FWTS_OK)
+		return;
 
 	/* Sanity check each field */
 	/* Battery State */
@@ -3794,7 +3801,7 @@ static void method_test_BST_return(
 			"be 0..7, got 0x%8.8" PRIx64 ".",
 			name, obj->Package.Elements[0].Integer.Value);
 		fwts_tag_failed(fw, FWTS_TAG_ACPI_METHOD_RETURN);
-		failed++;
+		failed = true;
 	}
 	/* Ensure bits 0 (discharging) and 1 (charging) are not both set, see 10.2.2.6 */
 	if (((obj->Package.Elements[0].Integer.Value) & 3) == 3) {
@@ -3805,7 +3812,7 @@ static void method_test_BST_return(
 			"which is not allowed. Got value 0x%8.8" PRIx64 ".",
 			name, obj->Package.Elements[0].Integer.Value);
 		fwts_tag_failed(fw, FWTS_TAG_ACPI_METHOD_RETURN);
-		failed++;
+		failed = true;
 	}
 	/* Battery Present Rate - cannot check, pulled from EC */
 	/* Battery Remaining Capacity - cannot check, pulled from EC */
@@ -3889,9 +3896,6 @@ static void method_test_BMD_return(
 	ACPI_OBJECT *obj,
 	void *private)
 {
-	uint32_t i;
-	int failed = 0;
-
 	FWTS_UNUSED(private);
 
 	if (method_check_type(fw, name, buf, ACPI_TYPE_PACKAGE) != FWTS_OK)
@@ -3900,22 +3904,12 @@ static void method_test_BMD_return(
 	if (method_package_count_equal(fw, name, "_BMD", obj, 5) != FWTS_OK)
 		return;
 
+	if (method_package_elements_all_type(fw, name, "_BMD", obj, ACPI_TYPE_INTEGER) != FWTS_OK)
+		return;
+
 	fwts_acpi_object_dump(fw, obj);
 
-	for (i= 0; (i < 4) && (i < obj->Package.Count); i++) {
-		if (obj->Package.Elements[i].Type != ACPI_TYPE_INTEGER) {
-			fwts_failed(fw, LOG_LEVEL_MEDIUM,
-				"Method_BMDBadType",
-				"%s package element %" PRIu32
-				" is not of type DWORD Integer.",
-				name, i);
-			fwts_tag_failed(fw, FWTS_TAG_ACPI_METHOD_RETURN);
-			failed++;
-		}
-	}
-	/* TODO: check return values */
-	if (!failed)
-		method_passed_sane(fw, name, "package");
+	method_passed_sane(fw, name, "package");
 }
 
 static int method_test_BMD(fwts_framework *fw)
@@ -4031,17 +4025,8 @@ static void method_test_FIF_return(
 	if (method_package_count_equal(fw, name, "_FIF", obj, 4) != FWTS_OK)
 		return;
 
-	fwts_acpi_object_dump(fw, obj);
-
-	if ((obj->Package.Elements[0].Type != ACPI_TYPE_INTEGER) ||
-	    (obj->Package.Elements[1].Type != ACPI_TYPE_INTEGER) ||
-	    (obj->Package.Elements[2].Type != ACPI_TYPE_INTEGER) ||
-	    (obj->Package.Elements[3].Type != ACPI_TYPE_INTEGER)) {
-		fwts_failed(fw, LOG_LEVEL_MEDIUM,
-			"Method_FIFBadType",
-			"%s should return package of 4 "
-			"integers.", name);
-		fwts_tag_failed(fw, FWTS_TAG_ACPI_METHOD_RETURN);
+	if (method_package_elements_all_type(fw, name, "_FIF",
+		obj, ACPI_TYPE_INTEGER) != FWTS_OK) {
 		fwts_advice(fw,
 			"%s is not returning the correct "
 			"fan information. It may be worth "
@@ -4049,8 +4034,12 @@ static void method_test_FIF_return(
 			"interactive 'fan' test to see if "
 			"this affects the control and "
 			"operation of the fan.", name);
-	} else
-		method_passed_sane(fw, name, "package");
+		return;
+	}
+
+	fwts_acpi_object_dump(fw, obj);
+
+	method_passed_sane(fw, name, "package");
 }
 
 static int method_test_FIF(fwts_framework *fw)
@@ -4084,15 +4073,8 @@ static void method_test_FST_return(
 	if (method_package_count_equal(fw, name, "_FST", obj, 3) != FWTS_OK)
 		return;
 
-	fwts_acpi_object_dump(fw, obj);
-	if ((obj->Package.Elements[0].Type != ACPI_TYPE_INTEGER) ||
-	    (obj->Package.Elements[1].Type != ACPI_TYPE_INTEGER) ||
-	    (obj->Package.Elements[2].Type != ACPI_TYPE_INTEGER)) {
-		fwts_failed(fw, LOG_LEVEL_MEDIUM,
-			"Method_FSTBadType",
-			"%s should return package of 3 "
-			"integers.", name);
-		fwts_tag_failed(fw, FWTS_TAG_ACPI_METHOD_RETURN);
+	if (method_package_elements_all_type(fw, name, "_FST",
+		obj, ACPI_TYPE_INTEGER) != FWTS_OK) {
 		fwts_advice(fw,
 			"%s is not returning the correct "
 			"fan status information. It may be "
@@ -4100,8 +4082,12 @@ static void method_test_FST_return(
 			"suite interactive 'fan' test to see "
 			"if this affects the control and "
 			"operation of the fan.", name);
-	} else
-		method_passed_sane(fw, name, "package");
+		return;
+	}
+
+	fwts_acpi_object_dump(fw, obj);
+
+	method_passed_sane(fw, name, "package");
 }
 
 static int method_test_FST(fwts_framework *fw)
@@ -4416,16 +4402,8 @@ static void method_test_WAK_return(
 	if (method_package_count_equal(fw, name, "_WAK", obj, 2) != FWTS_OK)
 		return;
 
-	if ((obj->Package.Elements[0].Type != ACPI_TYPE_INTEGER) ||
-	    (obj->Package.Elements[1].Type != ACPI_TYPE_INTEGER))  {
-		fwts_failed(fw, LOG_LEVEL_MEDIUM,
-			"Method_WAKBadType",
-			"%s should return package of 2 "
-			"integers, got %" PRIu32 " instead.",
-			name, obj->Package.Count);
-		fwts_tag_failed(fw, FWTS_TAG_ACPI_METHOD_RETURN);
+	if (method_package_elements_all_type(fw, name, "_WAK", obj, ACPI_TYPE_INTEGER) != FWTS_OK)
 		return;
-	}
 
 	method_passed_sane(fw, name, "package");
 }
@@ -4618,19 +4596,10 @@ static void method_test_BCL_return(
 	if (method_package_count_min(fw, name, "_BCL", obj, 3) != FWTS_OK)
 		return;
 
-	fwts_acpi_object_dump(fw, obj);
-
-	for (i = 0; i < obj->Package.Count; i++) {
-		if (obj->Package.Elements[i].Type != ACPI_TYPE_INTEGER)
-			failed++;
-	}
-	if (failed) {
-		fwts_failed(fw, LOG_LEVEL_MEDIUM,
-			"Method_BCLNoPackage",
-			"%s did not return a package of %" PRIu32
-			" integers.", name, obj->Package.Count);
+	if (method_package_elements_all_type(fw, name, "_FIF", obj, ACPI_TYPE_INTEGER) != FWTS_OK)
 		return;
-	}
+
+	fwts_acpi_object_dump(fw, obj);
 
 	if (obj->Package.Elements[0].Integer.Value <
 	    obj->Package.Elements[1].Integer.Value) {
-- 
1.8.0




More information about the fwts-devel mailing list