Re: [Qemu-devel] [PATCH 1/2] target-i386: Simplify listflags() function

[Paolo Bonzini <pbonzini@redhat.com>]

On 03/02/2015 19:08, Eduardo Habkost wrote:
> listflags() had lots of unnecessary complexity. Instead of printing to a
> buffer that will be immediately printed, simply call the printing
> function directly. Also, remove the fbits and flags arguments that were
> always set to the same value. Also, there's no need to list the flags in
> reverse order.
> 
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
>  target-i386/cpu.c | 42 ++++++++++++++----------------------------
>  1 file changed, 14 insertions(+), 28 deletions(-)
> 
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index 3a9b32e..39d2fda 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -1911,34 +1911,19 @@ static void x86_cpu_parse_featurestr(CPUState *cs, char *features,
>      }
>  }
>  
> -/* generate a composite string into buf of all cpuid names in featureset
> - * selected by fbits.  indicate truncation at bufsize in the event of overflow.
> - * if flags, suppress names undefined in featureset.
> +/* Print all cpuid feature names in featureset
>   */
> -static void listflags(char *buf, int bufsize, uint32_t fbits,
> -                      const char **featureset, uint32_t flags)
> -{
> -    const char **p = &featureset[31];
> -    char *q, *b, bit;
> -    int nc;
> -
> -    b = 4 <= bufsize ? buf + (bufsize -= 3) - 1 : NULL;
> -    *buf = '\0';
> -    for (q = buf, bit = 31; fbits && bufsize; --p, fbits &= ~(1 << bit), --bit)
> -        if (fbits & 1 << bit && (*p || !flags)) {
> -            if (*p)
> -                nc = snprintf(q, bufsize, "%s%s", q == buf ? "" : " ", *p);
> -            else
> -                nc = snprintf(q, bufsize, "%s[%d]", q == buf ? "" : " ", bit);
> -            if (bufsize <= nc) {
> -                if (b) {
> -                    memcpy(b, "...", sizeof("..."));
> -                }
> -                return;
> -            }
> -            q += nc;
> -            bufsize -= nc;
> +static void listflags(FILE *f, fprintf_function print, const char **featureset)
> +{
> +    int bit;
> +    bool first = true;
> +
> +    for (bit = 0; bit < 32; bit++) {
> +        if (featureset[bit]) {
> +            print(f, "%s%s", first ? "" : " ", featureset[bit]);
> +            first = false;
>          }
> +    }
>  }
>  
>  /* generate CPU information. */
> @@ -1963,8 +1948,9 @@ void x86_cpu_list(FILE *f, fprintf_function cpu_fprintf)
>      for (i = 0; i < ARRAY_SIZE(feature_word_info); i++) {
>          FeatureWordInfo *fw = &feature_word_info[i];
>  
> -        listflags(buf, sizeof(buf), (uint32_t)~0, fw->feat_names, 1);
> -        (*cpu_fprintf)(f, "  %s\n", buf);
> +        (*cpu_fprintf)(f, "  ");
> +        listflags(f, cpu_fprintf, fw->feat_names);
> +        (*cpu_fprintf)(f, "\n");
>      }
>  }
>  
> 

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>