[PATCH] acpi: method: add helper to check for package contents, simplify code.

Colin King colin.king at canonical.com
Thu Jan 10 19:10:13 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 and
method_package_elements_type() that checks the types against an
array of givem types.

These two helper functions allow us to remove a lot of duplicated code
and remove the need to do a lot of the error prone per object type
checking.

Also replace all "int failed" vars with booleans to simplify the code.

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

diff --git a/src/acpi/method/method.c b/src/acpi/method/method.c
index 21c89c0..7268495 100644
--- a/src/acpi/method/method.c
+++ b/src/acpi/method/method.c
@@ -476,7 +476,7 @@ static int method_evaluate_method(fwts_framework *fw,
 	fwts_list_link	*item;
 	fwts_list *methods;
 	size_t name_len = strlen(name);
-	int found = 0;
+	bool found = false;
 
 	if ((methods = fwts_acpi_object_get_names()) != NULL) {
 		fwts_list_foreach(item, methods) {
@@ -485,7 +485,7 @@ static int method_evaluate_method(fwts_framework *fw,
 			if (strncmp(name, method_name + len - name_len, name_len) == 0) {
 				ACPI_OBJECT_LIST  arg_list;
 
-				found++;
+				found = true;
 				arg_list.Count   = num_args;
 				arg_list.Pointer = args;
 				method_evaluate_found_method(fw, method_name,
@@ -537,7 +537,7 @@ static int method_name_check(fwts_framework *fw)
 {
 	fwts_list_link	*item;
 	fwts_list *methods;
-	int failed = 0;
+	bool failed = false;
 
  	if ((methods = fwts_acpi_object_get_names()) != NULL) {
 		fwts_log_info(fw, "Found %d Objects\n", methods->len);
@@ -559,7 +559,7 @@ static int method_name_check(fwts_framework *fw)
 						fwts_list_data(char *, item),
 						*ptr);
 					fwts_tag_failed(fw, FWTS_TAG_ACPI_METHOD);
-					failed++;
+					failed = true;
 					break;
 				}
 			}
@@ -746,6 +746,110 @@ static void method_test_polling_return(
 	}
 }
 
+#define ACPI_TYPE_INTBUF	(ACPI_TYPE_INVALID + 1)
+
+/*
+ *  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";
+	case ACPI_TYPE_INTBUF:
+		return "integer or buffer";
+	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;
+}
+
+typedef struct {
+	ACPI_OBJECT_TYPE type;	/* Type */
+	const char 	*name;	/* Field name */
+} fwts_package_element;
+
+/*
+ *  method_package_elements_type()
+ *	sanity check fields in a package that all have
+ *	the same type
+ */
+static int method_package_elements_type(
+	fwts_framework *fw,
+	const char *name,
+	const char *objname,
+	const ACPI_OBJECT *obj,
+	const fwts_package_element *info,
+	const uint32_t count)
+{
+	uint32_t i;
+	bool failed = false;
+	char tmp[128];
+
+	if (obj->Package.Count != count)
+		return FWTS_ERROR;
+
+	for (i = 0; i < obj->Package.Count; i++) {
+		if (obj->Package.Elements[i].Type != info[i].type) {
+			snprintf(tmp, sizeof(tmp), "Method%sElementType", objname);
+			fwts_failed(fw, LOG_LEVEL_MEDIUM, tmp,
+				"%s package element %" PRIu32 " (%s) was not the expected "
+				"type '%s', was instead type '%s'.",
+				name, i, info[i].name,
+				method_type_name(info[i].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;
+}
+
 /****************************************************************************/
 
 /*
@@ -899,29 +1003,16 @@ static void method_test_PLD_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 buffers */
-	for (i = 0; i < obj->Package.Count; i++) {
-		if (obj->Package.Elements[i].Type != ACPI_TYPE_BUFFER) {
-			fwts_failed(fw, LOG_LEVEL_MEDIUM,
-				"Method_PLDElementType",
-				"%s package element %" PRIu32 " was not a buffer.",
-				name, i);
-			fwts_tag_failed(fw, FWTS_TAG_ACPI_METHOD_RETURN);
-			failed = true;
-		}
-		/* We should sanity check the PLD further */
-	}
+	if (method_package_elements_all_type(fw, name, "_PLD", obj, ACPI_TYPE_BUFFER) != FWTS_OK)
+		return;
 
-	if (!failed)
-		method_passed_sane(fw, name, "package");
+	method_passed_sane(fw, name, "package");
 }
 
 static int method_test_PLD(fwts_framework *fw)
@@ -1752,26 +1843,21 @@ static void method_test_FIX_return(
 		return;
 
 	/* All elements in the package must be integers */
+	if (method_package_elements_all_type(fw, name, "_FIX", obj, ACPI_TYPE_INTEGER) != FWTS_OK)
+		return;
+
+	/* And they need to be valid IDs */
 	for (i = 0; i < obj->Package.Count; i++) {
-		if (obj->Package.Elements[i].Type != ACPI_TYPE_INTEGER) {
+		if (!method_valid_EISA_ID(
+			(uint32_t)obj->Package.Elements[i].Integer.Value,
+			tmp, sizeof(tmp))) {
 			fwts_failed(fw, LOG_LEVEL_MEDIUM,
-				"Method_FIXElementType",
-				"%s package element %" PRIu32 " was not an integer.",
-				name, i);
-			fwts_tag_failed(fw, FWTS_TAG_ACPI_METHOD_RETURN);
-		} else {
-			/* And they need to be valid IDs */
-			if (!method_valid_EISA_ID(
-				(uint32_t)obj->Package.Elements[i].Integer.Value,
-				tmp, sizeof(tmp))) {
-				fwts_failed(fw, LOG_LEVEL_MEDIUM,
-					"Method_FIXInvalidElementValue",
-					"%s returned an integer "
-					"0x%8.8" PRIx64 " in package element "
-					"%" PRIu32 " that is not a valid "
-					"EISA ID.", name, obj->Integer.Value, i);
-				failed = true;
-			}
+				"Method_FIXInvalidElementValue",
+				"%s returned an integer "
+				"0x%8.8" PRIx64 " in package element "
+				"%" PRIu32 " that is not a valid "
+				"EISA ID.", name, obj->Integer.Value, i);
+			failed = true;
 		}
 	}
 
@@ -1804,9 +1890,6 @@ static void method_test_HPP_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)
@@ -1817,19 +1900,10 @@ static void method_test_HPP_return(
 		return;
 
 	/* All 4 elements in the package must be integers */
-	for (i = 0; i < obj->Package.Count; i++) {
-		if (obj->Package.Elements[i].Type != ACPI_TYPE_INTEGER) {
-			fwts_failed(fw, LOG_LEVEL_MEDIUM,
-				"Method_HPPElementType",
-				"%s package element %" PRIu32 " was not an integer.",
-				name, i);
-			fwts_tag_failed(fw, FWTS_TAG_ACPI_METHOD_RETURN);
-			failed = true;
-		}
-	}
+	if (method_package_elements_all_type(fw, name, "_HPP", obj, ACPI_TYPE_INTEGER) != FWTS_OK)
+		return;
 
-	if (!failed)
-		method_passed_sane(fw, name, "package");
+	method_passed_sane(fw, name, "package");
 }
 
 static int method_test_HPP(fwts_framework *fw)
@@ -1855,28 +1929,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 +2165,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)						\
@@ -2244,7 +2296,7 @@ static void method_test_Sx__return(
 
 	/* Yes, we really want integers! */
 	if ((obj->Package.Elements[0].Type != ACPI_TYPE_INTEGER) ||
-	    (obj->Package.Elements[0].Type != ACPI_TYPE_INTEGER)) {
+	    (obj->Package.Elements[1].Type != ACPI_TYPE_INTEGER)) {
 		fwts_failed(fw, LOG_LEVEL_MEDIUM,
 			"Method_SxElementType",
 			"%s returned a package that did not contain "
@@ -2303,52 +2355,6 @@ static int method_test_SWS(fwts_framework *fw)
 /*
  * Section 8.4 Declaring Processors
  */
-static void method_test_type_integer(
-	fwts_framework *fw,
-	bool *failed,
-	ACPI_OBJECT *obj,
-	int element,
-	char *message)
-{
-	if (obj->Package.Elements[element].Type != ACPI_TYPE_INTEGER) {
-		fwts_failed(fw, LOG_LEVEL_MEDIUM, "Method_CPCElementType",
-			"_CPC package element %d (%s) was not an integer.",
-			element, message);
-		*failed = true;
-	}
-}
-
-static void method_test_type_buffer(
-	fwts_framework *fw,
-	bool *failed,
-	ACPI_OBJECT *obj,
-	int element,
-	char *message)
-{
-	if (obj->Package.Elements[element].Type != ACPI_TYPE_BUFFER) {
-		fwts_failed(fw, LOG_LEVEL_MEDIUM, "Method_CPCElementType",
-			"_CPC package element %d (%s) was not a buffer.",
-			element, message);
-		*failed = true;
-	}
-}
-
-static void method_test_type_mixed(
-	fwts_framework *fw,
-	bool *failed,
-	ACPI_OBJECT *obj,
-	int element,
-	char *message)
-{
-	if ((obj->Package.Elements[element].Type != ACPI_TYPE_BUFFER) &&
-	    (obj->Package.Elements[element].Type != ACPI_TYPE_BUFFER)) {
-		fwts_failed(fw, LOG_LEVEL_MEDIUM, "Method_CPCElementType",
-			"_CPC package element %d (%s) was not a buffer "
-			"or an integer.", element, message);
-		*failed = true;
-	}
-}
-
 static void method_test_CPC_return(
 	fwts_framework *fw,
 	char *name,
@@ -2356,7 +2362,25 @@ static void method_test_CPC_return(
 	ACPI_OBJECT *obj,
 	void *private)
 {
-	bool failed = false;
+	static fwts_package_element elements[] = {
+		{ ACPI_TYPE_INTEGER,	"Number of Entries" },
+		{ ACPI_TYPE_INTEGER,	"Revision" },
+		{ ACPI_TYPE_INTBUF,	"Highest Performance" },
+		{ ACPI_TYPE_INTBUF,	"Nominal Performance" },
+		{ ACPI_TYPE_INTBUF,	"Lowest Non Linear Performance" },
+		{ ACPI_TYPE_INTBUF,	"Lowest Performance" },
+		{ ACPI_TYPE_BUFFER,	"Guaranteed Performance Register" },
+		{ ACPI_TYPE_BUFFER,	"Desired Performance Register" },
+		{ ACPI_TYPE_BUFFER,	"Minimum Performance Register" },
+		{ ACPI_TYPE_BUFFER,	"Maximum Performance Register" },
+		{ ACPI_TYPE_BUFFER,	"Performance Reduction Tolerance Register" },
+		{ ACPI_TYPE_BUFFER,	"Timed Window Register" },
+		{ ACPI_TYPE_INTBUF,	"Counter Wraparound Time" },	
+		{ ACPI_TYPE_BUFFER,	"Nominal Counter Register" },
+		{ ACPI_TYPE_BUFFER,	"Delivered Counter Register" },
+		{ ACPI_TYPE_BUFFER,	"Performance Limited Register" },
+		{ ACPI_TYPE_BUFFER,	"Enable Register" }
+	};
 
 	FWTS_UNUSED(private);
 
@@ -2368,27 +2392,10 @@ static void method_test_CPC_return(
 		return;
 
 	/* For now, just check types */
+	if (method_package_elements_type(fw, name, "_CPC", obj, elements, 17) != FWTS_OK)
+		return;
 
-	method_test_type_integer(fw, &failed, obj, 0, "NumEntries");
-	method_test_type_integer(fw, &failed, obj, 1, "Revision");
-	method_test_type_mixed  (fw, &failed, obj, 2, "HighestPerformance");
-	method_test_type_mixed  (fw, &failed, obj, 3, "NominalPerformance");
-	method_test_type_mixed  (fw, &failed, obj, 4, "LowestNonlinerPerformance");
-	method_test_type_mixed  (fw, &failed, obj, 5, "LowestPerformance");
-	method_test_type_buffer (fw, &failed, obj, 6, "GuaranteedPerformanceRegister");
-	method_test_type_buffer (fw, &failed, obj, 7, "DesiredPerformanceRegister");
-	method_test_type_buffer (fw, &failed, obj, 8, "MinimumPerformanceRegister");
-	method_test_type_buffer (fw, &failed, obj, 9, "MaximumPerformanceRegister");
-	method_test_type_buffer (fw, &failed, obj, 10, "PerformanceReductionToleranceRegister");
-	method_test_type_buffer (fw, &failed, obj, 11, "TimeWindowRegister");
-	method_test_type_mixed  (fw, &failed, obj, 12, "CounterWraparoundTime");
-	method_test_type_mixed  (fw, &failed, obj, 13, "NominalCounterRegister");
-	method_test_type_mixed  (fw, &failed, obj, 14, "DeliveredCounterRegister");
-	method_test_type_mixed  (fw, &failed, obj, 15, "PerformanceLimitedRegister");
-	method_test_type_mixed  (fw, &failed, obj, 16, "EnableRegister");
-
-	if (!failed)
-		method_passed_sane(fw, name, "package");
+	method_passed_sane(fw, name, "package");
 }
 
 static int method_test_CPC(fwts_framework *fw)
@@ -2422,12 +2429,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 +2697,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 +2706,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)
@@ -3452,8 +3440,23 @@ static void method_test_BIF_return(
 	ACPI_OBJECT *obj,
 	void *private)
 {
-	uint32_t i;
-	int failed = 0;
+	bool failed = false;
+
+	static fwts_package_element elements[] = {
+		{ ACPI_TYPE_INTEGER,	"Power Unit" },
+		{ ACPI_TYPE_INTEGER,	"Design Capacity" },
+		{ ACPI_TYPE_INTEGER,	"Last Full Charge Capacity" },
+		{ ACPI_TYPE_INTEGER,	"Battery Technology" },
+		{ ACPI_TYPE_INTEGER,	"Design Voltage" },
+		{ ACPI_TYPE_INTEGER,	"Design Capacity of Warning" },
+		{ ACPI_TYPE_INTEGER,	"Design Capactty of Low" },
+		{ ACPI_TYPE_INTEGER,	"Battery Capacity Granularity 1" },
+		{ ACPI_TYPE_INTEGER,	"Battery Capacity Granularity 2" },
+		{ ACPI_TYPE_STRING,	"Model Number" },
+		{ ACPI_TYPE_STRING,	"Serial Number" },
+		{ ACPI_TYPE_STRING,	"Battery Type" },
+		{ ACPI_TYPE_STRING,	"OEM Information" }
+	};
 
 	FWTS_UNUSED(private);
 
@@ -3463,26 +3466,8 @@ static void method_test_BIF_return(
 	if (method_package_count_equal(fw, name, "_BIF", obj, 13) != FWTS_OK)
 		return;
 
-	for (i = 0; (i < 9) && (i < obj->Package.Count); i++) {
-		if (obj->Package.Elements[i].Type != ACPI_TYPE_INTEGER) {
-			fwts_failed(fw, LOG_LEVEL_MEDIUM,
-				"Method_BIFBadType",
-				"%s package element %" PRIu32
-				" is not of type DWORD Integer.", name, i);
-			fwts_tag_failed(fw, FWTS_TAG_ACPI_METHOD_RETURN);
-			failed++;
-		}
-	}
-	for (i = 9; (i < 13) && (i < obj->Package.Count); i++) {
-		if (obj->Package.Elements[i].Type != ACPI_TYPE_STRING) {
-			fwts_failed(fw, LOG_LEVEL_MEDIUM,
-				"Method_BIFBadType",
-				"%s package element %" PRIu32
-				" is not of type STRING.", name, i);
-			fwts_tag_failed(fw, FWTS_TAG_ACPI_METHOD_RETURN);
-			failed++;
-		}
-	}
+	if (method_package_elements_type(fw, name, "_BIF", obj, elements, 13) != FWTS_OK)
+		return;
 
 	/* Sanity check each field */
 	/* Power Unit */
@@ -3493,7 +3478,7 @@ static void method_test_BIF_return(
 			"0 (mWh) or 1 (mAh), got 0x%8.8" PRIx64 ".",
 			name, obj->Package.Elements[0].Integer.Value);
 		fwts_tag_failed(fw, FWTS_TAG_ACPI_METHOD_RETURN);
-		failed++;
+		failed = true;
 	}
 #ifdef FWTS_METHOD_PEDANDTIC
 	/*
@@ -3508,7 +3493,7 @@ static void method_test_BIF_return(
 			"unknown: 0x%8.8" PRIx64 ".",
 			name, obj->Package.Elements[1].Integer.Value);
 		fwts_tag_failed(fw, FWTS_TAG_ACPI_METHOD_RETURN);
-		failed++;
+		failed = true;
 	}
 	/* Last Full Charge Capacity */
 	if (obj->Package.Elements[2].Integer.Value > 0x7fffffff) {
@@ -3518,7 +3503,7 @@ static void method_test_BIF_return(
 			"is unknown: 0x%8.8" PRIx64 ".",
 			name, obj->Package.Elements[2].Integer.Value);
 		fwts_tag_failed(fw, FWTS_TAG_ACPI_METHOD_RETURN);
-		failed++;
+		failed = true;
 	}
 #endif
 	/* Battery Technology */
@@ -3530,7 +3515,7 @@ static void method_test_BIF_return(
 			"(Secondary), got 0x%8.8" PRIx64 ".",
 			name, obj->Package.Elements[3].Integer.Value);
 		fwts_tag_failed(fw, FWTS_TAG_ACPI_METHOD_RETURN);
-		failed++;
+		failed = true;
 	}
 #ifdef FWTS_METHOD_PEDANDTIC
 	/*
@@ -3545,7 +3530,7 @@ static void method_test_BIF_return(
 			"unknown: 0x%8.8" PRIx64 ".",
 			name, obj->Package.Elements[4].Integer.Value);
 		fwts_tag_failed(fw, FWTS_TAG_ACPI_METHOD_RETURN);
-		failed++;
+		failed = true;
 	}
 	/* Design capacity warning */
 	if (obj->Package.Elements[5].Integer.Value > 0x7fffffff) {
@@ -3555,7 +3540,7 @@ static void method_test_BIF_return(
 			"is unknown: 0x%8.8" PRIx64 ".",
 			name, obj->Package.Elements[5].Integer.Value);
 		fwts_tag_failed(fw, FWTS_TAG_ACPI_METHOD_RETURN);
-		failed++;
+		failed = true;
 	}
 	/* Design capacity low */
 	if (obj->Package.Elements[6].Integer.Value > 0x7fffffff) {
@@ -3565,7 +3550,7 @@ static void method_test_BIF_return(
 			"is unknown: 0x%8.8" PRIx64 ".",
 			name, obj->Package.Elements[6].Integer.Value);
 		fwts_tag_failed(fw, FWTS_TAG_ACPI_METHOD_RETURN);
-		failed++;
+		failed = true;
 	}
 #endif
 	if (failed)
@@ -3592,49 +3577,53 @@ static void method_test_BIX_return(
 	ACPI_OBJECT *obj,
 	void *private)
 {
-	uint32_t i;
-	int failed = 0;
+	bool failed = false;
+
+	static fwts_package_element elements[] = {
+		{ ACPI_TYPE_INTEGER,	"Revision" },
+		{ ACPI_TYPE_INTEGER,	"Power Unit" },
+		{ ACPI_TYPE_INTEGER,	"Design Capacity" },
+		{ ACPI_TYPE_INTEGER,	"Last Full Charge Capacity" },
+		{ ACPI_TYPE_INTEGER,	"Battery Technology" },
+		{ ACPI_TYPE_INTEGER,	"Design Voltage" },
+		{ ACPI_TYPE_INTEGER,	"Design Capacity of Warning" },
+		{ ACPI_TYPE_INTEGER,	"Design Capactty of Low" },
+		{ ACPI_TYPE_INTEGER,	"Cycle Count" },
+		{ ACPI_TYPE_INTEGER,	"Measurement Accuracy" },
+		{ ACPI_TYPE_INTEGER,	"Max Sampling Time" },
+		{ ACPI_TYPE_INTEGER,	"Min Sampling Time" },
+		{ ACPI_TYPE_INTEGER,	"Max Averaging Interval" },
+		{ ACPI_TYPE_INTEGER,	"Min Averaging Interval" },
+		{ ACPI_TYPE_INTEGER,	"Battery Capacity Granularity 1" },
+		{ ACPI_TYPE_INTEGER,	"Battery Capacity Granularity 2" },
+		{ ACPI_TYPE_STRING,	"Model Number" },
+		{ ACPI_TYPE_STRING,	"Serial Number" },
+		{ ACPI_TYPE_STRING,	"Battery Type" },
+		{ ACPI_TYPE_STRING,	"OEM Information" }
+	};
+
 	FWTS_UNUSED(private);
 
 	if (method_check_type(fw, name, buf, ACPI_TYPE_PACKAGE) != FWTS_OK)
 		return;
 
-	if (method_package_count_equal(fw, name, "_BIX", obj, 16) != FWTS_OK)
+	if (method_package_count_equal(fw, name, "_BIX", obj, 20) != FWTS_OK)
 		return;
 
-	for (i = 0; (i < 16) && (i < obj->Package.Count); i++) {
-		if (obj->Package.Elements[i].Type != ACPI_TYPE_INTEGER) {
-			fwts_failed(fw, LOG_LEVEL_MEDIUM,
-				"Method_BIXBadType",
-				"%s package element %" PRIu32
-				" is not of type DWORD Integer.",
-				name, i);
-			fwts_tag_failed(fw, FWTS_TAG_ACPI_METHOD_RETURN);
-			failed++;
-		}
-	}
-	for (i = 16; (i < 20) && (i < obj->Package.Count); i++) {
-		if (obj->Package.Elements[i].Type != ACPI_TYPE_STRING) {
-			fwts_failed(fw, LOG_LEVEL_MEDIUM,
-				"Method_BIXBadType",
-				"%s package element %" PRIu32
-				" is not of type STRING.",
-				name, i);
-			fwts_tag_failed(fw, FWTS_TAG_ACPI_METHOD_RETURN);
-			failed++;
-		}
-	}
+	if (method_package_elements_type(fw, name, "_BIX", obj, elements, 20) != FWTS_OK)
+		return;
 
 	/* Sanity check each field */
 	/* Power Unit */
 	if (obj->Package.Elements[1].Integer.Value > 0x00000002) {
 		fwts_failed(fw, LOG_LEVEL_MEDIUM,
 			"Method_BIXPowerUnit",
-			"%s: Expected Power Unit (Element 1) to be "
+			"%s: Expected %s (Element 1) to be "
 			"0 (mWh) or 1 (mAh), got 0x%8.8" PRIx64 ".",
-			name, obj->Package.Elements[1].Integer.Value);
+			name, elements[1].name,
+			obj->Package.Elements[1].Integer.Value);
 		fwts_tag_failed(fw, FWTS_TAG_ACPI_METHOD_RETURN);
-		failed++;
+		failed = true;
 	}
 #ifdef FWTS_METHOD_PEDANDTIC
 	/*
@@ -3645,33 +3634,36 @@ static void method_test_BIX_return(
 	if (obj->Package.Elements[2].Integer.Value > 0x7fffffff) {
 		fwts_failed(fw, LOG_LEVEL_LOW,
 			"Method_BIXDesignCapacity",
-			"%s: Design Capacity (Element 2) is "
+			"%s: %s (Element 2) is "
 			"unknown: 0x%8.8" PRIx64 ".",
-			name, obj->Package.Elements[2].Integer.Value);
+			name, elements[2].name,
+			obj->Package.Elements[2].Integer.Value);
 		fwts_tag_failed(fw, FWTS_TAG_ACPI_METHOD_RETURN);
-		failed++;
+		failed = true;
 	}
 	/* Last Full Charge Capacity */
 	if (obj->Package.Elements[3].Integer.Value > 0x7fffffff) {
 		fwts_failed(fw, LOG_LEVEL_LOW,
 			"Method_BIXFullChargeCapacity",
-			"%s: Last Full Charge Capacity (Element 3) "
+			"%s: %s (Element 3) "
 			"is unknown: 0x%8.8" PRIx64 ".",
-			name, obj->Package.Elements[3].Integer.Value);
+			name, elements[3].name,
+			obj->Package.Elements[3].Integer.Value);
 		fwts_tag_failed(fw, FWTS_TAG_ACPI_METHOD_RETURN);
-		failed++;
+		failed = true;
 	}
 #endif
 	/* Battery Technology */
 	if (obj->Package.Elements[4].Integer.Value > 0x00000002) {
 		fwts_failed(fw, LOG_LEVEL_MEDIUM,
 			"Method_BIXBatteryTechUnit",
-			"%s: Expected Battery Technology Unit "
+			"%s: %s "
 			"(Element 4) to be 0 (Primary) or 1 "
 			"(Secondary), got 0x%8.8" PRIx64 ".",
-			name, obj->Package.Elements[4].Integer.Value);
+			name, elements[4].name,
+			obj->Package.Elements[4].Integer.Value);
 		fwts_tag_failed(fw, FWTS_TAG_ACPI_METHOD_RETURN);
-		failed++;
+		failed = true;
 	}
 #ifdef FWTS_METHOD_PEDANDTIC
 	/*
@@ -3682,40 +3674,43 @@ static void method_test_BIX_return(
 	if (obj->Package.Elements[5].Integer.Value > 0x7fffffff) {
 		fwts_failed(fw, LOG_LEVEL_LOW,
 			"Method_BIXDesignVoltage",
-			"%s: Design Voltage (Element 5) is unknown: "
+			"%s: %s (Element 5) is unknown: "
 			"0x%8.8" PRIx64 ".",
-			name, obj->Package.Elements[5].Integer.Value);
+			name, elements[5].name,
+			obj->Package.Elements[5].Integer.Value);
 		fwts_tag_failed(fw, FWTS_TAG_ACPI_METHOD_RETURN);
-		failed++;
+		failed = true;
 	}
 	/* Design capacity warning */
 	if (obj->Package.Elements[6].Integer.Value > 0x7fffffff) {
 		fwts_failed(fw, LOG_LEVEL_LOW,
 			"Method_BIXDesignCapacityE6",
-			"%s: Design Capacity Warning (Element 6) "
+			"%s: %s (Element 6) "
 			"is unknown: 0x%8.8" PRIx64 ".",
-			name, obj->Package.Elements[6].Integer.Value);
+			name, elements[6].name,
+			obj->Package.Elements[6].Integer.Value);
 		fwts_tag_failed(fw, FWTS_TAG_ACPI_METHOD_RETURN);
-		failed++;
+		failed = true;
 	}
 	/* Design capacity low */
 	if (obj->Package.Elements[7].Integer.Value > 0x7fffffff) {
 		fwts_failed(fw, LOG_LEVEL_LOW,
 			"Method_BIXDesignCapacityE7",
-			"%s: Design Capacity Warning (Element 7) "
+			"%s: %s (Element 7) "
 			"is unknown: 0x%8.8" PRIx64 ".",
-			name, obj->Package.Elements[7].Integer.Value);
+			name, elements[7].name,
+			obj->Package.Elements[7].Integer.Value);
 		fwts_tag_failed(fw, FWTS_TAG_ACPI_METHOD_RETURN);
-		failed++;
+		failed = true;
 	}
 	/* Cycle Count */
-	if (obj->Package.Elements[10].Integer.Value > 0x7fffffff) {
+	if (obj->Package.Elements[8].Integer.Value > 0x7fffffff) {
 		fwts_failed(fw, LOG_LEVEL_LOW, "Method_BIXCyleCount",
-			"%s: Cycle Count (Element 10) is unknown: "
-			"0x%8.8" PRIx64 ".",
-			name, obj->Package.Elements[10].Integer.Value);
+			"%s: %s (Element 8) is unknown: "
+			"0x%8.8" PRIx64 ".", Elements[8].name,
+			name, obj->Package.Elements[8].Integer.Value);
 		fwts_tag_failed(fw, FWTS_TAG_ACPI_METHOD_RETURN);
-		failed++;
+		failed = true;
 	}
 #endif
 	if (failed)
@@ -3762,8 +3757,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 +3767,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 +3779,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 +3790,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 +3874,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 +3882,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)
@@ -3981,6 +3953,15 @@ static void method_test_PIF_return(
 	ACPI_OBJECT *obj,
 	void *private)
 {
+	static fwts_package_element elements[] = {
+		{ ACPI_TYPE_INTEGER,	"Power Source State" },
+		{ ACPI_TYPE_INTEGER,	"Maximum Output Power" },
+		{ ACPI_TYPE_INTEGER,	"Maximum Input Power" },
+		{ ACPI_TYPE_STRING,	"Model Number" },
+		{ ACPI_TYPE_STRING,	"Serial Number" },
+		{ ACPI_TYPE_STRING,	"OEM Information" }
+	};
+
 	FWTS_UNUSED(private);
 
 	if (method_check_type(fw, name, buf, ACPI_TYPE_PACKAGE) != FWTS_OK)
@@ -3989,21 +3970,12 @@ static void method_test_PIF_return(
 	if (method_package_count_equal(fw, name, "_PIF", obj, 6) != FWTS_OK)
 		return;
 
+	if (method_package_elements_type(fw, name, "_PIF", obj, elements, 6) != FWTS_OK)
+		return;
+
 	fwts_acpi_object_dump(fw, obj);
 
-	if ((obj->Package.Elements[0].Type != ACPI_TYPE_BUFFER) ||
-	    (obj->Package.Elements[1].Type != ACPI_TYPE_INTEGER) ||
-	    (obj->Package.Elements[2].Type != ACPI_TYPE_INTEGER) ||
-	    (obj->Package.Elements[3].Type != ACPI_TYPE_STRING) ||
-	    (obj->Package.Elements[4].Type != ACPI_TYPE_STRING) ||
-	    (obj->Package.Elements[5].Type != ACPI_TYPE_STRING)) {
-		fwts_failed(fw, LOG_LEVEL_MEDIUM,
-			"Method_PIFBadType",
-			"%s should return package of 1 "
-			"buffer, 2 integers and 3 strings.", name);
-		fwts_tag_failed(fw, FWTS_TAG_ACPI_METHOD_RETURN);
-	} else
-		method_passed_sane(fw, name, "package");
+	method_passed_sane(fw, name, "package");
 }
 
 static int method_test_PIF(fwts_framework *fw)
@@ -4031,17 +4003,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 +4012,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 +4051,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 +4060,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 +4380,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");
 }
@@ -4474,7 +4430,8 @@ static void method_test_DOD_return(
 	void *private)
 {
 	uint32_t i;
-	int failed = 0;
+	bool failed = false;
+
 	static char *dod_type[] = {
 		"Other",
 		"VGA, CRT or VESA Compatible Analog Monitor",
@@ -4504,7 +4461,7 @@ static void method_test_DOD_return(
 
 	for (i = 0; i < obj->Package.Count; i++) {
 		if (obj->Package.Elements[i].Type != ACPI_TYPE_INTEGER)
-			failed++;
+			failed = true;
 		else {
 			uint32_t val = obj->Package.Elements[i].Integer.Value;
 			fwts_log_info_verbatum(fw, "Device %" PRIu32 ":", i);
@@ -4607,7 +4564,7 @@ static void method_test_BCL_return(
 	void *private)
 {
 	uint32_t i;
-	int failed = 0;
+	bool failed = false;
 	bool ascending_levels = false;
 
 	FWTS_UNUSED(private);
@@ -4618,19 +4575,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, "_BCL", 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) {
@@ -4643,7 +4591,7 @@ static void method_test_BCL_return(
 			obj->Package.Elements[0].Integer.Value,
 			obj->Package.Elements[1].Integer.Value);
 		fwts_tag_failed(fw, FWTS_TAG_ACPI_METHOD_RETURN);
-		failed++;
+		failed = true;
 	}
 
 	for (i = 2; i < obj->Package.Count - 1; i++) {
@@ -4658,7 +4606,7 @@ static void method_test_BCL_return(
 				obj->Package.Elements[i].Integer.Value, i,
 				obj->Package.Elements[i+1].Integer.Value, i+1);
 			ascending_levels = true;
-			failed++;
+			failed = true;
 		}
 	}
 	if (ascending_levels) {
@@ -4669,6 +4617,7 @@ static void method_test_BCL_return(
 			"order which should be fixed "
 			"in the firmware.");
 		fwts_tag_failed(fw, FWTS_TAG_ACPI_METHOD_RETURN);
+		failed = true;
 	}
 
 	if (failed)
-- 
1.8.0




More information about the fwts-devel mailing list