From patchwork Thu Jan 10 19:10:13 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Colin Ian King X-Patchwork-Id: 211121 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from chlorine.canonical.com (chlorine.canonical.com [91.189.94.204]) by ozlabs.org (Postfix) with ESMTP id 570CA2C00BF for ; Fri, 11 Jan 2013 06:10:20 +1100 (EST) Received: from localhost ([127.0.0.1] helo=chlorine.canonical.com) by chlorine.canonical.com with esmtp (Exim 4.71) (envelope-from ) id 1TtNW7-0006hh-5U; Thu, 10 Jan 2013 19:10:19 +0000 Received: from youngberry.canonical.com ([91.189.89.112]) by chlorine.canonical.com with esmtp (Exim 4.71) (envelope-from ) id 1TtNW3-0006hR-T5 for fwts-devel@lists.ubuntu.com; Thu, 10 Jan 2013 19:10:15 +0000 Received: from cpc3-craw6-2-0-cust180.croy.cable.virginmedia.com ([77.100.248.181] helo=localhost) by youngberry.canonical.com with esmtpsa (TLS1.0:DHE_RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1TtNW2-0004ZZ-IX for fwts-devel@lists.ubuntu.com; Thu, 10 Jan 2013 19:10:15 +0000 From: Colin King To: fwts-devel@lists.ubuntu.com Subject: [PATCH] acpi: method: add helper to check for package contents, simplify code. Date: Thu, 10 Jan 2013 19:10:13 +0000 Message-Id: <1357845013-32388-1-git-send-email-colin.king@canonical.com> X-Mailer: git-send-email 1.8.0 X-BeenThere: fwts-devel@lists.ubuntu.com X-Mailman-Version: 2.1.13 Precedence: list List-Id: Firmware Test Suite Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: fwts-devel-bounces@lists.ubuntu.com Errors-To: fwts-devel-bounces@lists.ubuntu.com From: Colin Ian King 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 Acked-by: Keng-Yu Lin Acked-by: Ivan Hu --- 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)