Re: [Qemu-devel] [PATCH 18/22] target-i386: parse cpu_model string into set of stringified properties

[Don Slutz <Don@CloudSwitch.Com>]

On 09/07/12 16:55, Igor Mammedov wrote:
> cpu_model string does represent features in following format:
>   ([+-]feat)|(feat=foo)|(feat)
> which makes it impossible directly use property infrastructure
> to set features on CPU.
> This patch introduces parser that splits CPU name from cpu_model and
> converts legacy features string into canonized set of strings that
> is compatible with property manipulation infrastructure.
>
> PS:
>    * later it could be used as a hook to convert legacy command line
>      features to global properties. Then marked as deprecated and
>      removed with -cpu option in the future.
>    * compiler complains that it's unused function but I guess it is
>      easier for review this way
>
> Signed-off-by: Igor Mammedov <imammedo@redhat.com>
> ---
>   target-i386/cpu.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 52 insertions(+)
>
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index 0543e62..2c9cd6b 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -1332,6 +1332,58 @@ static void cpudef_2_x86_cpu(X86CPU *cpu, x86_def_t *def, Error **errp)
>       env->cpuid_xlevel2 = def->xlevel2;
>   }
>   
> +/* convert legacy cpumodel string to string cpu_name and
> + * a uniforms set of custom features that will be applied to CPU
> + * using object_property_parse()
> + */
> +static void compat_normalize_cpu_model(const char *cpu_model, char **cpu_name,
> +                                        QDict **features, Error **errp)
> +{
> +
> +    char *s = g_strdup(cpu_model);
> +    char *featurestr, *sptr;
> +
> +    *cpu_name = strtok_r(s, ",", &sptr);
I get:

cc1: warnings being treated as errors
/root/qemu-cpu-v2/target-i386/cpu.c: In function 'cpu_x86_register':
/root/qemu-cpu-v2/target-i386/cpu.c:1341: error: 'sptr' may be used 
uninitialized in this function
/root/qemu-cpu-v2/target-i386/cpu.c:1341: note: 'sptr' was declared here

And the change:

diff --git a/target-i386/cpu.c b/target-i386/cpu.c
index e7964a3..af50a8f 100644
--- a/target-i386/cpu.c
+++ b/target-i386/cpu.c
@@ -1338,7 +1338,7 @@ static void compat_normalize_cpu_model(const char 
*cpu_model, char **cpu_name,
  {

      char *s = g_strdup(cpu_model);
-    char *featurestr, *sptr;
+    char *featurestr, *sptr = NULL;

      *cpu_name = strtok_r(s, ",", &sptr);
      *features = qdict_new();

fixes this for me.

> +    *features = qdict_new();
> +
> +    featurestr = strtok_r(NULL, ",", &sptr);
> +    while (featurestr) {
> +        char *val;
> +        if (featurestr[0] == '+') {
> +            /*
> +             * preseve legacy behaviour, if feature was disabled once
> +             * do not allow to enable it again
> +             */
> +            if (!qdict_haskey(*features, featurestr + 1)) {
> +                qdict_put(*features, featurestr + 1, qstring_from_str("on"));
> +            }
> +        } else if (featurestr[0] == '-') {
> +            qdict_put(*features, featurestr + 1, qstring_from_str("off"));
> +        } else {
> +            val = strchr(featurestr, '=');
> +            if (val) {
> +                *val = 0; val++;
> +                if (!strcmp(featurestr, "vendor")) {
> +                    qdict_put(*features, "vendor-override",
> +                              qstring_from_str("on"));
> +                    qdict_put(*features, featurestr, qstring_from_str(val));
> +                } else if (!strcmp(featurestr, "tsc_freq")) {
> +                    qdict_put(*features, "tsc-frequency",
> +                              qstring_from_str(val));
> +                } else {
> +                    qdict_put(*features, featurestr, qstring_from_str(val));
> +                }
> +            } else {
> +                qdict_put(*features, featurestr, qstring_from_str("on"));
> +            }
> +        }
> +
> +        featurestr = strtok_r(NULL, ",", &sptr);
> +    }
> +
> +    return;
> +}
> +
>   static int cpu_x86_find_by_name(X86CPU *cpu, x86_def_t *x86_cpu_def,
>                                   const char *cpu_model, Error **errp)
>   {
  -Don Slutz