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

From: Colin Ian King <colin.king@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@canonical.com>
---
 src/acpi/method/method.c | 655 ++++++++++++++++++++++-------------------------
 1 file changed, 302 insertions(+), 353 deletions(-)

On Fri, Jan 11, 2013 at 3:10 AM, Colin King <colin.king@canonical.com> wrote:
> From: Colin Ian King <colin.king@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@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
>
Acked-by: Keng-Yu Lin <kengyu@canonical.com>

On 01/11/2013 03:10 AM, Colin King wrote:
> From: Colin Ian King <colin.king@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@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)
>

Acked-by: Ivan Hu <ivan.hu@canonical.com>