On Wed, Jun 01, 2016 at 06:37:26PM +0200, Igor Mammedov wrote: > Signed-off-by: Igor Mammedov <imamm...@redhat.com> > --- > target-i386/cpu.c | 32 +++++++++++++++++--------------- > 1 file changed, 17 insertions(+), 15 deletions(-) > > diff --git a/target-i386/cpu.c b/target-i386/cpu.c > index 238f69d..618aef9 100644 > --- a/target-i386/cpu.c > +++ b/target-i386/cpu.c > @@ -1945,12 +1945,15 @@ static void x86_cpu_parse_featurestr(CPUState *cs, > char *features, > X86CPU *cpu = X86_CPU(cs); > char *featurestr; /* Single 'key=value" string being parsed */ > uint32_t numvalue; > + char num[32]; > + const char *name; > + char *err; > Error *local_err = NULL; > > featurestr = features ? strtok(features, ",") : NULL; > > while (featurestr) { > - char *val; > + char *val = NULL; > if (featurestr[0] == '+') { > add_flagname_to_bitmaps(featurestr + 1, plus_features, > &local_err); > } else if (featurestr[0] == '-') { > @@ -1958,10 +1961,8 @@ static void x86_cpu_parse_featurestr(CPUState *cs, > char *features, > } else if ((val = strchr(featurestr, '='))) { > *val = 0; val++; > feat2prop(featurestr); > + name = featurestr; > if (!strcmp(featurestr, "xlevel")) { > - char *err; > - char num[32]; > - > numvalue = strtoul(val, &err, 0); > if (!*val || *err) { > error_setg(errp, "bad numerical value %s", val); > @@ -1973,11 +1974,9 @@ static void x86_cpu_parse_featurestr(CPUState *cs, > char *features, > numvalue += 0x80000000; > } > snprintf(num, sizeof(num), "%" PRIu32, numvalue); > - object_property_parse(OBJECT(cpu), num, featurestr, > &local_err); > + val = num; > } else if (!strcmp(featurestr, "tsc-freq")) { > int64_t tsc_freq; > - char *err; > - char num[32]; > > tsc_freq = qemu_strtosz_suffix_unit(val, &err, > QEMU_STRTOSZ_DEFSUFFIX_B, > 1000); > @@ -1986,12 +1985,11 @@ static void x86_cpu_parse_featurestr(CPUState *cs, > char *features, > return; > } > snprintf(num, sizeof(num), "%" PRId64, tsc_freq); > - object_property_parse(OBJECT(cpu), num, "tsc-frequency", > - &local_err); > + val = num; > + name = "tsc-frequency"; > } else if (!strcmp(featurestr, "hv-spinlocks")) { > - char *err; > const int min = 0xFFF; > - char num[32]; > + > numvalue = strtoul(val, &err, 0); > if (!*val || *err) { > error_setg(errp, "bad numerical value %s", val); > @@ -2004,14 +2002,18 @@ static void x86_cpu_parse_featurestr(CPUState *cs, > char *features, > numvalue = min; > } > snprintf(num, sizeof(num), "%" PRId32, numvalue); > - object_property_parse(OBJECT(cpu), num, featurestr, > &local_err); > - } else { > - object_property_parse(OBJECT(cpu), val, featurestr, > &local_err); > + val = num; > } > } else { > feat2prop(featurestr); > - object_property_parse(OBJECT(cpu), "on", featurestr, &local_err); > + name = featurestr; > + val = (char *)"on";
You don't need this hack if you do something like this: while (featurestr) { - char *val = NULL; + const char *val = NULL; + char *eq = NULL; if (featurestr[0] == '+') { add_flagname_to_bitmaps(featurestr + 1, plus_features, &local_err); } else if (featurestr[0] == '-') { add_flagname_to_bitmaps(featurestr + 1, minus_features, &local_err); - } else if ((val = strchr(featurestr, '='))) { - *val = 0; val++; + } else if ((eq = strchr(featurestr, '='))) { + *eq++ = 0; + val = eq; feat2prop(featurestr); name = featurestr; if (!strcmp(featurestr, "xlevel")) { > } > + > + if (val) { > + object_property_parse(OBJECT(cpu), val, name, &local_err); > + } I would find it easier to read if this part was inside the same block as the code that sets name/val. I would find it even better if object_property_parse() was in the main loop body, and the +feature/-feature code placed inside an if/continue branch (to make it clear that it is an exception, not the rule). Like this: /* untested */ static void x86_cpu_parse_featurestr(CPUState *cs, char *features, Error **errp) { X86CPU *cpu = X86_CPU(cs); char *featurestr; /* Single 'key=value" string being parsed */ char *err; Error *local_err = NULL; if (!features) { return; } for (featurestr = strtok(features, ","); featurestr; featurestr = strtok(NULL, ",")) { const char *name; const char *val = NULL; char *eq = NULL; uint32_t numvalue; char num[32]; /* Compatibility syntax: */ if (featurestr[0] == '+') { add_flagname_to_bitmaps(featurestr + 1, plus_features, &local_err); continue; } else if (featurestr[0] == '-') { add_flagname_to_bitmaps(featurestr + 1, minus_features, &local_err); continue; } eq = strchr(featurestr, '='); if (eq) { *eq++ = 0; val = eq; } else { val = "on"; } feat2prop(featurestr); name = featurestr; /* Special cases: */ if (!strcmp(name, "xlevel")) { numvalue = strtoul(val, &err, 0); if (!*val || *err) { error_setg(errp, "bad numerical value %s", val); return; } if (numvalue < 0x80000000) { error_report("xlevel value shall always be >= 0x80000000" ", fixup will be removed in future versions"); numvalue += 0x80000000; } snprintf(num, sizeof(num), "%" PRIu32, numvalue); val = num; } else if (!strcmp(name, "tsc-freq")) { int64_t tsc_freq; tsc_freq = qemu_strtosz_suffix_unit(val, &err, QEMU_STRTOSZ_DEFSUFFIX_B, 1000); if (tsc_freq < 0 || *err) { error_setg(errp, "bad numerical value %s", val); return; } snprintf(num, sizeof(num), "%" PRId64, tsc_freq); val = num; name = "tsc-frequency"; } else if (!strcmp(name, "hv-spinlocks")) { const int min = 0xFFF; numvalue = strtoul(val, &err, 0); if (!*val || *err) { error_setg(errp, "bad numerical value %s", val); return; } if (numvalue < min) { error_report("hv-spinlocks value shall always be >= 0x%x" ", fixup will be removed in future versions", min); numvalue = min; } snprintf(num, sizeof(num), "%" PRId32, numvalue); val = num; } object_property_parse(OBJECT(cpu), val, name, &local_err); if (local_err) { error_propagate(errp, local_err); return; } } } > + > if (local_err) { > error_propagate(errp, local_err); > return; > -- > 1.8.3.1 > -- Eduardo