[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-arm] [PATCH 05/10] target-i386: cpu: consolidate calls of obje
From: |
Eduardo Habkost |
Subject: |
Re: [Qemu-arm] [PATCH 05/10] target-i386: cpu: consolidate calls of object_property_parse() in x86_cpu_parse_featurestr |
Date: |
Tue, 7 Jun 2016 17:37:48 -0300 |
User-agent: |
Mutt/1.6.1 (2016-04-27) |
On Mon, Jun 06, 2016 at 05:16:47PM +0200, Igor Mammedov wrote:
> From: Eduardo Habkost <address@hidden>
>
> Signed-off-by: Eduardo Habkost <address@hidden>
> Reviewed-by: Igor Mammedov <address@hidden>
> Signed-off-by: Igor Mammedov <address@hidden>
Reviewed-by: Eduardo Habkost <address@hidden>
A small suggestion in case we are going to need a new version of
this series, below:
> ---
> v1:
> - fix error handling in of +-feat, Igor Mammedov <address@hidden>
> - rebase on top of
> "target-i386: Remove xlevel & hv-spinlocks option fixups"
> ---
> target-i386/cpu.c | 74
> ++++++++++++++++++++++++++++++++++++-------------------
> 1 file changed, 49 insertions(+), 25 deletions(-)
>
> diff --git a/target-i386/cpu.c b/target-i386/cpu.c
> index 349b971..f791a06 100644
> --- a/target-i386/cpu.c
> +++ b/target-i386/cpu.c
> @@ -1957,43 +1957,67 @@ static void x86_cpu_parse_featurestr(CPUState *cs,
> char *features,
> char *featurestr; /* Single 'key=value" string being parsed */
> Error *local_err = NULL;
>
> - featurestr = features ? strtok(features, ",") : NULL;
> + if (!features) {
> + return;
> + }
>
> - while (featurestr) {
> - char *val;
> + for (featurestr = strtok(features, ",");
> + featurestr;
> + featurestr = strtok(NULL, ",")) {
> + const char *name;
> + const char *val = NULL;
> + char *eq = NULL;
> +
> + /* Compatibility syntax: */
> if (featurestr[0] == '+') {
> add_flagname_to_bitmaps(featurestr + 1, plus_features,
> &local_err);
> + if (local_err) {
> + error_propagate(errp, local_err);
> + return;
> + }
> + continue;
If you add an error_propagate() call to the end of the function,
this can be shortened to:
if (local_err) {
break;
}
Or maybe the loop could be simply written as:
for (featurestr = strtok(features, ",");
featurestr && !local_err;
featurestr = strtok(NULL, ","))
and we could avoid all the if (local_err) checks inside the loop
body.
> } else if (featurestr[0] == '-') {
> add_flagname_to_bitmaps(featurestr + 1, minus_features,
> &local_err);
> - } else if ((val = strchr(featurestr, '='))) {
> - *val = 0; val++;
> - feat2prop(featurestr);
> - 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);
> - if (tsc_freq < 0 || *err) {
> - error_setg(errp, "bad numerical value %s", val);
> - return;
> - }
> - snprintf(num, sizeof(num), "%" PRId64, tsc_freq);
> - object_property_parse(OBJECT(cpu), num, "tsc-frequency",
> - &local_err);
> - } else {
> - object_property_parse(OBJECT(cpu), val, featurestr,
> &local_err);
> + if (local_err) {
> + error_propagate(errp, local_err);
> + return;
> }
> + continue;
> + }
> +
> + eq = strchr(featurestr, '=');
> + if (eq) {
> + *eq++ = 0;
> + val = eq;
> } else {
> - feat2prop(featurestr);
> - object_property_parse(OBJECT(cpu), "on", featurestr, &local_err);
> + val = "on";
> + }
> +
> + feat2prop(featurestr);
> + name = featurestr;
> +
> + /* Special case: */
> + if (!strcmp(name, "tsc-freq")) {
> + int64_t tsc_freq;
> + char *err;
> + char num[32];
> +
> + 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";
> }
> +
> + object_property_parse(OBJECT(cpu), val, name, &local_err);
> if (local_err) {
> error_propagate(errp, local_err);
> return;
> }
> - featurestr = strtok(NULL, ",");
> }
> }
>
> --
> 1.8.3.1
>
--
Eduardo
[Qemu-arm] [PATCH 05/10] target-i386: cpu: consolidate calls of object_property_parse() in x86_cpu_parse_featurestr, Igor Mammedov, 2016/06/06
- Re: [Qemu-arm] [PATCH 05/10] target-i386: cpu: consolidate calls of object_property_parse() in x86_cpu_parse_featurestr,
Eduardo Habkost <=
[Qemu-arm] [PATCH 07/10] target-sparc: cpu: use sparc_cpu_parse_features() directly, Igor Mammedov, 2016/06/06
[Qemu-arm] [PATCH 09/10] arm: virt: parse cpu_model only once, Igor Mammedov, 2016/06/06