Re: [Qemu-devel] [PATCH v3 3/3] Change other funcitons referring to feature_word_info[]

[Eric Blake]

On 08/10/2018 09:06 AM, Robert Hoo wrote:

In the subject: s/funcitons/functions/

Also, it may be worth using a topic prefix (most of our commit messages 
resemble:

topic: Description of patch

to make it easier to spot patches by topic).

> Add an util function feature_word_description(), which help construct the string

s/an util/a util/
s/help/helps/

> describing the feature word (both CPUID and MSR types).
>
> report_unavailable_features(): add MSR_FEATURE_WORD type support.
> x86_cpu_get_feature_words(): limit to CPUID_FEATURE_WORD only.
> x86_cpu_get_supported_feature_word(): add MSR_FEATURE_WORD type support.
> x86_cpu_adjust_feat_level(): assert the requested feature must be
> CPUID_FEATURE_WORD type.
>
> Signed-off-by: Robert Hoo <address@hidden>
> ---
>   target/i386/cpu.c | 77 +++++++++++++++++++++++++++++++++++++++++--------------
>   1 file changed, 58 insertions(+), 19 deletions(-)
>
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index f7c70d9..21ed599 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -3024,21 +3024,51 @@ static const TypeInfo host_x86_cpu_type_info = {
>   
>   #endif
>   
> +/*
> +*caller should have input str no less than 64 byte length.
> +*/
> +#define FEATURE_WORD_DESCPTION_LEN 64

s/DESCPTION/DESCRIPTION/

> +static int feature_word_description(char str[], FeatureWordInfo *f,
> +                                            uint32_t bit)
> +{

Prone to buffer overflow if the caller doesn't pass in the length. 
Would it be better to just g_strdup_printf() into a malloc'd result 
instead of trying to snprintf()'ing into a buffer that you hope the 
caller sized large enough?

> +    int ret;
> +
> +    assert(f->type == CPUID_FEATURE_WORD ||
> +        f->type == MSR_FEATURE_WORD);
> +    switch (f->type) {
> +    case CPUID_FEATURE_WORD:
> +        {
> +            const char *reg = get_register_name_32(f->cpuid.reg);
> +            assert(reg);
> +            ret = snprintf(str, FEATURE_WORD_DESCPTION_LEN,
> +                "CPUID.%02XH:%s%s%s [bit %d]",
> +                f->cpuid.eax, reg,
> +                f->feat_names[bit] ? "." : "",
> +                f->feat_names[bit] ? f->feat_names[bit] : "", bit);
> +            break;
> +        }
> +    case MSR_FEATURE_WORD:
> +        ret = snprintf(str, FEATURE_WORD_DESCPTION_LEN,
> +            "MSR(%xH).%s [bit %d]",
> +            f->msr.index,
> +            f->feat_names[bit] ? f->feat_names[bit] : "", bit);
> +        break;
> +    }

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3266
Virtualization:  qemu.org | libvirt.org