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

login
register
mail settings
Submitter Colin King
Date Jan. 10, 2013, 7:10 p.m.
Message ID <1357845013-32388-1-git-send-email-colin.king@canonical.com>
Download mbox | patch
Permalink /patch/211121/
State Accepted
Headers show

Comments

Colin King - Jan. 10, 2013, 7:10 p.m.
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(-)
Keng-Yu Lin - Jan. 29, 2013, 7:06 a.m.
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>
Ivan Hu - Jan. 31, 2013, 10:02 a.m.
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>

Patch

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)