[PATCH 4/7 v3] fwts: Print names of missing features, rather than a cryptic bitmask

[Keng-Yu Lin]

On Thu, Jul 24, 2014 at 5:07 PM, Jeremy Kerr <jk at ozlabs.org> wrote:
> This change adds a function to return a comma-separated list of features
> in a bitmask, which we use when printing missing firmware features when
> we skip a test.
>
> Signed-off-by: Jeremy Kerr <jk at ozlabs.org>
>
> ---
>  src/lib/include/fwts_firmware.h |    6 ++--
>  src/lib/src/fwts_firmware.c     |   48 ++++++++++++++++++++++++++++++++
>  src/lib/src/fwts_framework.c    |    4 +-
>  3 files changed, 54 insertions(+), 4 deletions(-)
>
> diff --git a/src/lib/include/fwts_firmware.h b/src/lib/include/fwts_firmware.h
> index 265ef4d..cd57acc 100644
> --- a/src/lib/include/fwts_firmware.h
> +++ b/src/lib/include/fwts_firmware.h
> @@ -29,13 +29,15 @@ enum firmware_type {
>  };
>
>  enum firmware_feature {
> -       FWTS_FW_FEATURE_ACPI = 1 << 0,
> +       FWTS_FW_FEATURE_ACPI            = 0x1,
> +       FWTS_FW_FEATURE_ALL             = FWTS_FW_FEATURE_ACPI
>  };
>
>  int fwts_firmware_detect(void);
>  int fwts_firmware_features(void);
> +const char *fwts_firmware_feature_string(const int features);
>
> -static inline bool fwts_firmware_has_features(int features)
> +static inline bool fwts_firmware_has_features(const int features)
>  {
>         return (fwts_firmware_features() & features) == features;
>  }
> diff --git a/src/lib/src/fwts_firmware.c b/src/lib/src/fwts_firmware.c
> index 4b59cef..8723906 100644
> --- a/src/lib/src/fwts_firmware.c
> +++ b/src/lib/src/fwts_firmware.c
> @@ -26,6 +26,13 @@
>  static enum firmware_type firmware_type;
>  static bool firmware_type_valid;
>
> +static struct {
> +       enum firmware_feature feature;
> +       const char name[16];
> +} feature_names[] = {
> +       { FWTS_FW_FEATURE_ACPI,         "ACPI" },
> +};
> +
>  /*
>   *  fwts_memory_map_entry_compare()
>   *     callback used to sort memory_map entries on start address
> @@ -64,3 +71,44 @@ int fwts_firmware_features(void)
>
>         return features;
>  }
> +
> +const char *fwts_firmware_feature_string(const int features)
> +{
> +       const int n = FWTS_ARRAY_LEN(feature_names);
> +       const char sep[] = ", ";
> +       static char str[50];
> +       size_t len;
> +       char *p;
> +       int i;
> +
> +       /* ensure we have enough space in str to store n names, plus n-1
> +        * separators, plus a trailing nul */
> +       FWTS_ASSERT((n * (sizeof(feature_names[0].name) - 1)) +
> +                               ((n-1) * (sizeof(sep) - 1)) + 1 <
> +                       sizeof(str), str_too_small);
> +
> +       /* ensure we have a name defined for all features */
> +       FWTS_ASSERT(((1 << n) - 1) == FWTS_FW_FEATURE_ALL,
> +                       invalid_feature_names);
> +
> +       for (p = str, i = 0; i < n; i++) {
> +
> +               if (!(features & feature_names[i].feature))
> +                       continue;
> +
> +               /* if this isn't the first feature, add a separator */
> +               if (p != str) {
> +                       len = sizeof(sep) - 1;
> +                       memcpy(p, sep, len);
> +                       p += len;
> +               }
> +
> +               len = strlen(feature_names[i].name);
> +               memcpy(p, feature_names[i].name, len);
> +               p += len;
> +       }
> +
> +       *p = '\0';
> +
> +       return str;
> +}
> diff --git a/src/lib/src/fwts_framework.c b/src/lib/src/fwts_framework.c
> index d200d26..669a944 100644
> --- a/src/lib/src/fwts_framework.c
> +++ b/src/lib/src/fwts_framework.c
> @@ -507,8 +507,8 @@ static int fwts_framework_run_test(fwts_framework *fw, fwts_framework_test *test
>
>         if (!fwts_firmware_has_features(test->fw_features)) {
>                 int missing = test->fw_features & ~fwts_firmware_features();
> -               fwts_log_info(fw, "Test skipped, missing features 0x%08x",
> -                               missing);
> +               fwts_log_info(fw, "Test skipped, missing features: %s",
> +                               fwts_firmware_feature_string(missing));
>                 fw->current_major_test->results.skipped +=
>                         test->ops->total_tests;
>                 fw->total.skipped += test->ops->total_tests;
>
> --


Acked-by: Keng-Yu Lin <kengyu at canonical.com>