[PATCH] acpi: method: tidy up some more tests

Keng-Yu Lin kengyu at canonical.com
Tue Jan 29 05:46:19 UTC 2013


On Thu, Jan 10, 2013 at 7:52 PM, Colin King <colin.king at canonical.com> wrote:
> From: Colin Ian King <colin.king at canonical.com>
>
> This patch tidies up some of the tests by checking for failures
> early on and bailing out of the test early.  This means we can
> remove one level of if statement nesting which makes the code
> less convoluted.
>
> Also remove some unwanted extra whitespaces.
>
> Signed-off-by: Colin Ian King <colin.king at canonical.com>
> ---
>  src/acpi/method/method.c | 159 ++++++++++++++++++++++++-----------------------
>  1 file changed, 81 insertions(+), 78 deletions(-)
>
> diff --git a/src/acpi/method/method.c b/src/acpi/method/method.c
> index baf9c36..21c89c0 100644
> --- a/src/acpi/method/method.c
> +++ b/src/acpi/method/method.c
> @@ -1937,25 +1937,26 @@ static void method_test_STA_return(
>
>         FWTS_UNUSED(private);
>
> -       if (method_check_type(fw, name, buf, ACPI_TYPE_INTEGER) == FWTS_OK) {
> -               if ((obj->Integer.Value & 3) == 2) {
> -                       fwts_failed(fw, LOG_LEVEL_MEDIUM,
> -                               "Method_STAEnabledNotPresent",
> -                               "%s indicates that the device is enabled "
> -                               "but not present, which is impossible.", name);
> -                       failed = true;
> -               }
> -               if ((obj->Integer.Value & ~0x1f) != 0) {
> -                       fwts_failed(fw, LOG_LEVEL_MEDIUM,
> -                               "Method_STAReservedBitsSet",
> -                               "%s is returning non-zero reserved "
> -                               "bits 5-31. These should be zero.", name);
> -                       failed = true;
> -               }
> +       if (method_check_type(fw, name, buf, ACPI_TYPE_INTEGER) != FWTS_OK)
> +               return;
>
> -               if (!failed)
> -                       method_passed_sane_uint64(fw, name, obj->Integer.Value);
> +       if ((obj->Integer.Value & 3) == 2) {
> +               fwts_failed(fw, LOG_LEVEL_MEDIUM,
> +                       "Method_STAEnabledNotPresent",
> +                       "%s indicates that the device is enabled "
> +                       "but not present, which is impossible.", name);
> +               failed = true;
> +       }
> +       if ((obj->Integer.Value & ~0x1f) != 0) {
> +               fwts_failed(fw, LOG_LEVEL_MEDIUM,
> +                       "Method_STAReservedBitsSet",
> +                       "%s is returning non-zero reserved "
> +                       "bits 5-31. These should be zero.", name);
> +               failed = true;
>         }
> +
> +       if (!failed)
> +               method_passed_sane_uint64(fw, name, obj->Integer.Value);
>  }
>
>  static int method_test_STA(fwts_framework *fw)
> @@ -2011,18 +2012,19 @@ static void method_test_SEG_return(
>  {
>         FWTS_UNUSED(private);
>
> -       if (method_check_type(fw, name, buf, ACPI_TYPE_INTEGER) == FWTS_OK) {
> -               if ((obj->Integer.Value & 0xffff0000)) {
> -                       fwts_failed(fw, LOG_LEVEL_MEDIUM,
> -                               "Method_SEGIllegalReserved",
> -                               "%s returned value 0x%8.8" PRIx64 " and some of the "
> -                               "upper 16 reserved bits are set when they "
> -                               "should in fact be zero.",
> -                               name, obj->Integer.Value);
> -                       fwts_tag_failed(fw, FWTS_TAG_ACPI_METHOD_RETURN);
> -               } else
> -                       method_passed_sane_uint64(fw, name, obj->Integer.Value);
> -       }
> +       if (method_check_type(fw, name, buf, ACPI_TYPE_INTEGER) != FWTS_OK)
> +               return;
> +
> +       if ((obj->Integer.Value & 0xffff0000)) {
> +               fwts_failed(fw, LOG_LEVEL_MEDIUM,
> +                       "Method_SEGIllegalReserved",
> +                       "%s returned value 0x%8.8" PRIx64 " and some of the "
> +                       "upper 16 reserved bits are set when they "
> +                       "should in fact be zero.",
> +                       name, obj->Integer.Value);
> +               fwts_tag_failed(fw, FWTS_TAG_ACPI_METHOD_RETURN);
> +       } else
> +               method_passed_sane_uint64(fw, name, obj->Integer.Value);
>  }
>
>  static int method_test_SEG(fwts_framework *fw)
> @@ -2308,7 +2310,7 @@ static void method_test_type_integer(
>         int element,
>         char *message)
>  {
> -       if (obj->Package.Elements[element].Type  != ACPI_TYPE_INTEGER) {
> +       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);
> @@ -2323,7 +2325,7 @@ static void method_test_type_buffer(
>         int element,
>         char *message)
>  {
> -       if (obj->Package.Elements[element].Type  != ACPI_TYPE_BUFFER) {
> +       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);
> @@ -2338,8 +2340,8 @@ static void method_test_type_mixed(
>         int element,
>         char *message)
>  {
> -       if ((obj->Package.Elements[element].Type  != ACPI_TYPE_BUFFER) &&
> -           (obj->Package.Elements[element].Type  != ACPI_TYPE_BUFFER)) {
> +       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);
> @@ -3235,19 +3237,19 @@ static void method_test_GCP_return(
>  {
>         FWTS_UNUSED(private);
>
> -       if (method_check_type(fw, name, buf, ACPI_TYPE_INTEGER) == FWTS_OK) {
> -               if (obj->Integer.Value & ~0xf) {
> -                       fwts_failed(fw, LOG_LEVEL_MEDIUM,
> -                               "Method_GCPReturn",
> -                               "%s returned %" PRId64 ", should be between 0 and 15, "
> -                               "one or more of the reserved bits 4..31 seem "
> -                               "to be set.",
> -                               name, obj->Integer.Value);
> -                       fwts_tag_failed(fw, FWTS_TAG_ACPI_METHOD_RETURN);
> -               } else {
> -                       method_passed_sane_uint64(fw, name, obj->Integer.Value);
> -               }
> -       }
> +       if (method_check_type(fw, name, buf, ACPI_TYPE_INTEGER) != FWTS_OK)
> +               return;
> +
> +       if (obj->Integer.Value & ~0xf) {
> +               fwts_failed(fw, LOG_LEVEL_MEDIUM,
> +                       "Method_GCPReturn",
> +                       "%s returned %" PRId64 ", should be between 0 and 15, "
> +                       "one or more of the reserved bits 4..31 seem "
> +                       "to be set.",
> +                       name, obj->Integer.Value);
> +               fwts_tag_failed(fw, FWTS_TAG_ACPI_METHOD_RETURN);
> +       } else
> +               method_passed_sane_uint64(fw, name, obj->Integer.Value);
>  }
>
>  static int method_test_GCP(fwts_framework *fw)
> @@ -3300,19 +3302,19 @@ static void method_test_GWS_return(
>  {
>         FWTS_UNUSED(private);
>
> -       if (method_check_type(fw, name, buf, ACPI_TYPE_INTEGER) == FWTS_OK) {
> -               if (obj->Integer.Value & ~0x3) {
> -                       fwts_failed(fw, LOG_LEVEL_MEDIUM,
> -                               "Method_GWSReturn",
> -                               "%s returned %" PRIu64 ", should be between 0 and 3, "
> -                               "one or more of the reserved bits 2..31 seem "
> -                               "to be set.",
> -                               name, obj->Integer.Value);
> -                       fwts_tag_failed(fw, FWTS_TAG_ACPI_METHOD_RETURN);
> -               } else {
> -                       method_passed_sane_uint64(fw, name, obj->Integer.Value);
> -               }
> -       }
> +       if (method_check_type(fw, name, buf, ACPI_TYPE_INTEGER) != FWTS_OK)
> +               return;
> +
> +       if (obj->Integer.Value & ~0x3) {
> +               fwts_failed(fw, LOG_LEVEL_MEDIUM,
> +                       "Method_GWSReturn",
> +                       "%s returned %" PRIu64 ", should be between 0 and 3, "
> +                       "one or more of the reserved bits 2..31 seem "
> +                       "to be set.",
> +                       name, obj->Integer.Value);
> +               fwts_tag_failed(fw, FWTS_TAG_ACPI_METHOD_RETURN);
> +       } else
> +               method_passed_sane_uint64(fw, name, obj->Integer.Value);
>  }
>
>  static int method_test_GWS(fwts_framework *fw)
> @@ -3395,25 +3397,26 @@ static void method_test_SBS_return(
>
>         FWTS_UNUSED(private);
>
> -       if (method_check_type(fw, name, buf, ACPI_TYPE_INTEGER) == FWTS_OK) {
> -               switch (obj->Integer.Value) {
> -               case 0 ... 4:
> -                       fwts_passed(fw, "%s correctly returned value %" PRIu64 " %s",
> -                               name, obj->Integer.Value,
> -                               sbs_info[obj->Integer.Value]);
> -                       break;
> -               default:
> -                       fwts_failed(fw, LOG_LEVEL_MEDIUM, "Method_SBSReturn",
> -                               "%s returned %" PRIu64 ", should be between 0 and 4.",
> -                               name, obj->Integer.Value);
> -                       fwts_tag_failed(fw, FWTS_TAG_ACPI_METHOD_RETURN);
> -                       fwts_advice(fw,
> -                               "Smart Battery %s is incorrectly informing "
> -                               "the OS about the smart battery "
> -                               "configuration. This is a bug and needs to be "
> -                               "fixed.", name);
> -                       break;
> -               }
> +       if (method_check_type(fw, name, buf, ACPI_TYPE_INTEGER) != FWTS_OK)
> +               return;
> +
> +       switch (obj->Integer.Value) {
> +       case 0 ... 4:
> +               fwts_passed(fw, "%s correctly returned value %" PRIu64 " %s",
> +                       name, obj->Integer.Value,
> +                       sbs_info[obj->Integer.Value]);
> +               break;
> +       default:
> +               fwts_failed(fw, LOG_LEVEL_MEDIUM, "Method_SBSReturn",
> +                       "%s returned %" PRIu64 ", should be between 0 and 4.",
> +                       name, obj->Integer.Value);
> +               fwts_tag_failed(fw, FWTS_TAG_ACPI_METHOD_RETURN);
> +               fwts_advice(fw,
> +                       "Smart Battery %s is incorrectly informing "
> +                       "the OS about the smart battery "
> +                       "configuration. This is a bug and needs to be "
> +                       "fixed.", name);
> +               break;
>         }
>  }
>
> --
> 1.8.0
>
Acked-by: Keng-Yu Lin <kengyu at canonical.com>



More information about the fwts-devel mailing list