$SUBJ: "qemu_monitor_json: Introduce qemuMonitorJSONBuildCPUModelInfoFromJSON" On 04/19/2018 12:06 AM, Chris Venteicher wrote:
New function qemuMonitorJSONBuildCPUModelInfoFromJSON created by extracting code from existing function qemuMonitorJSONGetCPUModelExpansion to create a reusable function for extracting cpu model info from json.
Function qemuMonitorJSONGetCPUModelInfoFromJSON returns qemuMonitorCPUModelInfoPtr on success and NULL on failure.
Then in here, just indicate: Extract cpu_model parsing code from qemuMonitorJSONGetCPUModelExpansion into a separate helper for future reuse.
--- src/qemu/qemu_monitor_json.c | 78 +++++++++++++++++++++++++++++--------------- 1 file changed, 52 insertions(+), 26 deletions(-)
diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 57c2c4de0..4368aaaa0 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -5337,6 +5337,57 @@ qemuMonitorJSONParseCPUModelProperty(const char *key, return 0; }
Use 2 blank/empty lines between functions - mostly a readability thing.
+/* model_json: {"model": {"name": "IvyBridge", "props": {}}} + */ +static qemuMonitorCPUModelInfoPtr +qemuMonitorJSONBuildCPUModelInfoFromJSON(virJSONValuePtr model_json)
The '_json' is probably redundant.
+{ + virJSONValuePtr cpu_model; + virJSONValuePtr cpu_props; + qemuMonitorCPUModelInfoPtr machine_model = NULL; + qemuMonitorCPUModelInfoPtr model = NULL; + char const *cpu_name; + + if (!(cpu_model = virJSONValueObjectGetObject(model_json, "model"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("query-cpu-model-expansion reply data was missing 'model'"));
As a different suggestion from Collin... Since qemuMonitorJSONGetCPUModelExpansion already fetches @cpu_model, why not just pass virJSONValuePtr cpu_model. Then in patch3, fetch cpu_model but use the error for the specific command. then...
+ goto cleanup; + } + + if (!(cpu_name = virJSONValueObjectGetString(cpu_model, "name"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("query-cpu-model-expansion reply data was missing 'name'"));
s/query-cpu-model-expansion/cpu_model/
+ goto cleanup; + } + + if (!(cpu_props = virJSONValueObjectGetObject(cpu_model, "props"))) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("query-cpu-model-expansion reply data was missing 'props'"));
s/query-cpu-model-expansion/cpu_model/
+ goto cleanup; + } + + if (VIR_ALLOC(machine_model) < 0) + goto cleanup; + + if (VIR_STRDUP(machine_model->name, cpu_name) < 0) + goto cleanup; + + if (VIR_ALLOC_N(machine_model->props, virJSONValueObjectKeysNumber(cpu_props)) < 0) + goto cleanup; + + if (virJSONValueObjectForeachKeyValue(cpu_props, + qemuMonitorJSONParseCPUModelProperty, + machine_model) < 0) + goto cleanup; + + VIR_STEAL_PTR(model, machine_model); + + cleanup: + qemuMonitorCPUModelInfoFree(machine_model); + + return model; +} + int qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon, qemuMonitorCPUModelExpansionType type, @@ -5351,9 +5402,7 @@ qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon, virJSONValuePtr reply = NULL; virJSONValuePtr data; virJSONValuePtr cpu_model; - virJSONValuePtr cpu_props; qemuMonitorCPUModelInfoPtr machine_model = NULL; - char const *cpu_name; const char *typeStr = "";
*model_info = NULL; @@ -5426,30 +5475,7 @@ qemuMonitorJSONGetCPUModelExpansion(qemuMonitorPtr mon, goto retry; }
- if (!(cpu_name = virJSONValueObjectGetString(cpu_model, "name"))) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("query-cpu-model-expansion reply data was missing 'name'")); - goto cleanup; - } - - if (!(cpu_props = virJSONValueObjectGetObject(cpu_model, "props"))) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("query-cpu-model-expansion reply data was missing 'props'")); - goto cleanup; - } - - if (VIR_ALLOC(machine_model) < 0) - goto cleanup; - - if (VIR_STRDUP(machine_model->name, cpu_name) < 0) - goto cleanup; - - if (VIR_ALLOC_N(machine_model->props, virJSONValueObjectKeysNumber(cpu_props)) < 0) - goto cleanup; - - if (virJSONValueObjectForeachKeyValue(cpu_props, - qemuMonitorJSONParseCPUModelProperty, - machine_model) < 0) + if (!(machine_model = qemuMonitorJSONBuildCPUModelInfoFromJSON(data))) goto cleanup;
ret = 0;
Feel free to do the VIR_STEAL_PTR(*model_info, machine_model) or if (!(*machine_info = ...())) and get rid of machine_model from here. John