[Mesa-dev] [PATCH v2] aubinator: Store enum textual name in iter->value.

[Lionel Landwerlin]

Haha, nice :)

Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin at intel.com>

On 20/03/17 16:26, Kenneth Graunke wrote:
> gen_field_iterator_next() produces a string representing the value of
> the field.  For enum values, it also produced a separate "description"
> string containing the textual name of the enum.
>
> The only caller of this function combines the two, printing enums as
> "<numeric value> (<texture enum name>)".  We may as well just store
> that in item->value directly, eliminating the description field, and
> a layer of wrapping.
>
> v2: Use non-overlapping source and destination strings in snprintf.
> ---
>   src/intel/tools/aubinator.c |  7 +------
>   src/intel/tools/decoder.c   | 26 ++++++++++++++------------
>   src/intel/tools/decoder.h   |  1 -
>   3 files changed, 15 insertions(+), 19 deletions(-)
>
> Good catch - I wasn't aware of that restriction.  How about this
> instead?
>
> diff --git a/src/intel/tools/aubinator.c b/src/intel/tools/aubinator.c
> index fe6127ee312..6a37da11650 100644
> --- a/src/intel/tools/aubinator.c
> +++ b/src/intel/tools/aubinator.c
> @@ -116,12 +116,7 @@ print_iterator_values(struct gen_field_iterator *iter, int *idx)
>   {
>       char *token = NULL;
>       if (strstr(iter->value, "struct") == NULL) {
> -       if (strlen(iter->description) > 0) {
> -          fprintf(outfile, "    %s: %s (%s)\n",
> -                 iter->name, iter->value, iter->description);
> -       } else {
> -          fprintf(outfile, "    %s: %s\n", iter->name, iter->value);
> -       }
> +       fprintf(outfile, "    %s: %s\n", iter->name, iter->value);
>       } else {
>           token = strtok(iter->value, " ");
>           if (token != NULL) {
> diff --git a/src/intel/tools/decoder.c b/src/intel/tools/decoder.c
> index 72913601c04..ac3f135527e 100644
> --- a/src/intel/tools/decoder.c
> +++ b/src/intel/tools/decoder.c
> @@ -726,16 +726,15 @@ gen_field_iterator_init(struct gen_field_iterator *iter,
>      iter->print_colors = print_colors;
>   }
>   
> -static void
> -gen_enum_write_value(char *str, size_t max_length,
> -                      struct gen_enum *e, uint64_t value)
> +static const char *
> +gen_get_enum_name(struct gen_enum *e, uint64_t value)
>   {
>      for (int i = 0; i < e->nvalues; i++) {
>         if (e->values[i]->value == value) {
> -         strncpy(str, e->values[i]->name, max_length);
> -         return;
> +         return e->values[i]->name;
>         }
>      }
> +   return NULL;
>   }
>   
>   bool
> @@ -759,7 +758,7 @@ gen_field_iterator_next(struct gen_field_iterator *iter)
>      else
>         v.qw = iter->p[index];
>   
> -   iter->description[0] = '\0';
> +   const char *enum_name = NULL;
>   
>      switch (f->type.kind) {
>      case GEN_TYPE_UNKNOWN:
> @@ -767,16 +766,14 @@ gen_field_iterator_next(struct gen_field_iterator *iter)
>         uint64_t value = field(v.qw, f->start, f->end);
>         snprintf(iter->value, sizeof(iter->value),
>                  "%"PRId64, value);
> -      gen_enum_write_value(iter->description, sizeof(iter->description),
> -                           &f->inline_enum, value);
> +      enum_name = gen_get_enum_name(&f->inline_enum, value);
>         break;
>      }
>      case GEN_TYPE_UINT: {
>         uint64_t value = field(v.qw, f->start, f->end);
>         snprintf(iter->value, sizeof(iter->value),
>                  "%"PRIu64, value);
> -      gen_enum_write_value(iter->description, sizeof(iter->description),
> -                            &f->inline_enum, value);
> +      enum_name = gen_get_enum_name(&f->inline_enum, value);
>         break;
>      }
>      case GEN_TYPE_BOOL: {
> @@ -812,11 +809,16 @@ gen_field_iterator_next(struct gen_field_iterator *iter)
>         uint64_t value = field(v.qw, f->start, f->end);
>         snprintf(iter->value, sizeof(iter->value),
>                  "%"PRId64, value);
> -      gen_enum_write_value(iter->description, sizeof(iter->description),
> -                           f->type.gen_enum, value);
> +      enum_name = gen_get_enum_name(f->type.gen_enum, value);
>         break;
>      }
>      }
>   
> +   if (enum_name) {
> +      int length = strlen(iter->value);
> +      snprintf(iter->value + length, sizeof(iter->value) - length,
> +               " (%s)", enum_name);
> +   }
> +
>      return true;
>   }
> diff --git a/src/intel/tools/decoder.h b/src/intel/tools/decoder.h
> index 9f0aa4f35f1..b17be1d5fbf 100644
> --- a/src/intel/tools/decoder.h
> +++ b/src/intel/tools/decoder.h
> @@ -54,7 +54,6 @@ struct gen_field_iterator {
>      struct gen_group *group;
>      const char *name;
>      char value[128];
> -   char description[128];
>      const uint32_t *p;
>      int i;
>      bool print_colors;