[PATCH 00/13] String list handling cleanup in qemu capabilities code
Peter Krempa (13): qemuMonitorJSONParsePropsList: Refactor cleanup qemuMonitorJSONGetObjectProps: Refactor cleanup qemuMonitorJSONGetMigrationCapabilities: Refactor cleanup qemuMonitorJSONGetCommands: Refactor cleanup qemuMonitorJSONGetStringArray: Refactor cleanup qemuMonitorJSONGetObjectTypes: Refactor cleanup testQemuMonitorJSONGetCommands: Refactor cleanup testQemuMonitorJSONGetTPMModels: Refactor cleanup qemu: capabilities: Use g_auto(GStrv) instead of virStringListFreeCount virQEMUCapsProcessStringFlags: Don't require 'nvalues' virQEMUCapsProbeQMPTPM: Refactor handling of string lists qemuMonitorJSONGetStringArray: Don't return element count qemuMonitorJSONGetStringListProperty: Don't return element count src/qemu/qemu_capabilities.c | 82 ++++++++------------- src/qemu/qemu_monitor_json.c | 138 ++++++++++++----------------------- tests/qemumonitorjsontest.c | 46 ++++-------- 3 files changed, 96 insertions(+), 170 deletions(-) -- 2.31.1
Use 'g_auto' for @proplist and remove @ret. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_monitor_json.c | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 46aa3330a8..3e7a61c52c 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -6615,14 +6615,13 @@ qemuMonitorJSONParsePropsList(virJSONValue *cmd, char ***props) { virJSONValue *data; - char **proplist = NULL; + g_auto(GStrv) proplist = NULL; size_t n = 0; size_t count = 0; size_t i; - int ret = -1; if (qemuMonitorJSONCheckReply(cmd, reply, VIR_JSON_TYPE_ARRAY) < 0) - goto cleanup; + return -1; data = virJSONValueObjectGetArray(reply, "return"); n = virJSONValueArraySize(data); @@ -6641,18 +6640,14 @@ qemuMonitorJSONParsePropsList(virJSONValue *cmd, if (!(tmp = virJSONValueObjectGetString(child, "name"))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("reply data was missing 'name'")); - goto cleanup; + return -1; } proplist[count++] = g_strdup(tmp); } - ret = count; *props = g_steal_pointer(&proplist); - - cleanup: - g_strfreev(proplist); - return ret; + return count; } -- 2.31.1
Use 'g_autoptr' for the two temporary JSON objects and remove the cleanup section. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_monitor_json.c | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 3e7a61c52c..64696da210 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -6709,9 +6709,8 @@ qemuMonitorJSONGetObjectProps(qemuMonitor *mon, const char *object, char ***props) { - int ret = -1; - virJSONValue *cmd; - virJSONValue *reply = NULL; + g_autoptr(virJSONValue) cmd = NULL; + g_autoptr(virJSONValue) reply = NULL; *props = NULL; @@ -6721,18 +6720,12 @@ qemuMonitorJSONGetObjectProps(qemuMonitor *mon, return -1; if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) - goto cleanup; + return -1; - if (qemuMonitorJSONHasError(reply, "DeviceNotFound")) { - ret = 0; - goto cleanup; - } + if (qemuMonitorJSONHasError(reply, "DeviceNotFound")) + return 0; - ret = qemuMonitorJSONParsePropsList(cmd, reply, NULL, props); - cleanup: - virJSONValueFree(reply); - virJSONValueFree(cmd); - return ret; + return qemuMonitorJSONParsePropsList(cmd, reply, NULL, props); } -- 2.31.1
Use automatic memory clearing and remove the cleanup section. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_monitor_json.c | 29 ++++++++++------------------- 1 file changed, 10 insertions(+), 19 deletions(-) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 64696da210..f0d67f8f20 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -6768,11 +6768,10 @@ int qemuMonitorJSONGetMigrationCapabilities(qemuMonitor *mon, char ***capabilities) { - int ret = -1; - virJSONValue *cmd; - virJSONValue *reply = NULL; + g_autoptr(virJSONValue) cmd = NULL; + g_autoptr(virJSONValue) reply = NULL; virJSONValue *caps; - char **list = NULL; + g_auto(GStrv) list = NULL; size_t i; size_t n; @@ -6783,15 +6782,13 @@ qemuMonitorJSONGetMigrationCapabilities(qemuMonitor *mon, return -1; if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) - goto cleanup; + return -1; - if (qemuMonitorJSONHasError(reply, "CommandNotFound")) { - ret = 0; - goto cleanup; - } + if (qemuMonitorJSONHasError(reply, "CommandNotFound")) + return 0; if (qemuMonitorJSONCheckReply(cmd, reply, VIR_JSON_TYPE_ARRAY) < 0) - goto cleanup; + return -1; caps = virJSONValueObjectGetArray(reply, "return"); n = virJSONValueArraySize(caps); @@ -6805,26 +6802,20 @@ qemuMonitorJSONGetMigrationCapabilities(qemuMonitor *mon, if (!cap || virJSONValueGetType(cap) != VIR_JSON_TYPE_OBJECT) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("missing entry in migration capabilities list")); - goto cleanup; + return -1; } if (!(name = virJSONValueObjectGetString(cap, "capability"))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("missing migration capability name")); - goto cleanup; + return -1; } list[i] = g_strdup(name); } - ret = n; *capabilities = g_steal_pointer(&list); - - cleanup: - g_strfreev(list); - virJSONValueFree(cmd); - virJSONValueFree(reply); - return ret; + return n; } -- 2.31.1
Use automatic memory freeing to simplify the control flow. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_monitor_json.c | 22 +++++++--------------- 1 file changed, 7 insertions(+), 15 deletions(-) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index f0d67f8f20..683b389670 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -6159,11 +6159,10 @@ qemuMonitorJSONGetCPUModelComparison(qemuMonitor *mon, int qemuMonitorJSONGetCommands(qemuMonitor *mon, char ***commands) { - int ret = -1; - virJSONValue *cmd; - virJSONValue *reply = NULL; + g_autoptr(virJSONValue) cmd = NULL; + g_autoptr(virJSONValue) reply = NULL; virJSONValue *data; - char **commandlist = NULL; + g_auto(GStrv) commandlist = NULL; size_t n = 0; size_t i; @@ -6173,10 +6172,10 @@ int qemuMonitorJSONGetCommands(qemuMonitor *mon, return -1; if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) - goto cleanup; + return -1; if (qemuMonitorJSONCheckReply(cmd, reply, VIR_JSON_TYPE_ARRAY) < 0) - goto cleanup; + return -1; data = virJSONValueObjectGetArray(reply, "return"); n = virJSONValueArraySize(data); @@ -6191,21 +6190,14 @@ int qemuMonitorJSONGetCommands(qemuMonitor *mon, if (!(tmp = virJSONValueObjectGetString(child, "name"))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("query-commands reply data was missing 'name'")); - goto cleanup; + return -1; } commandlist[i] = g_strdup(tmp); } - ret = n; *commands = g_steal_pointer(&commandlist); - - - cleanup: - g_strfreev(commandlist); - virJSONValueFree(cmd); - virJSONValueFree(reply); - return ret; + return n; } -- 2.31.1
Use automatic memory clearing to simplify the control flow. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_monitor_json.c | 27 ++++++++++----------------- 1 file changed, 10 insertions(+), 17 deletions(-) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 683b389670..c25e282d51 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -7201,12 +7201,12 @@ qemuMonitorJSONBlockExportAdd(qemuMonitor *mon, static int -qemuMonitorJSONGetStringArray(qemuMonitor *mon, const char *qmpCmd, +qemuMonitorJSONGetStringArray(qemuMonitor *mon, + const char *qmpCmd, char ***array) { - int ret = -1; - virJSONValue *cmd; - virJSONValue *reply = NULL; + g_autoptr(virJSONValue) cmd = NULL; + g_autoptr(virJSONValue) reply = NULL; *array = NULL; @@ -7214,25 +7214,18 @@ qemuMonitorJSONGetStringArray(qemuMonitor *mon, const char *qmpCmd, return -1; if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) - goto cleanup; + return -1; - if (qemuMonitorJSONHasError(reply, "CommandNotFound")) { - ret = 0; - goto cleanup; - } + if (qemuMonitorJSONHasError(reply, "CommandNotFound")) + return 0; if (qemuMonitorJSONCheckReply(cmd, reply, VIR_JSON_TYPE_ARRAY) < 0) - goto cleanup; + return -1; if (!(*array = virJSONValueObjectGetStringArray(reply, "return"))) - goto cleanup; - - ret = g_strv_length(*array); + return -1; - cleanup: - virJSONValueFree(cmd); - virJSONValueFree(reply); - return ret; + return g_strv_length(*array); } int qemuMonitorJSONGetTPMModels(qemuMonitor *mon, -- 2.31.1
Use automatic memory clearing to simplify the control flow. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_monitor_json.c | 26 ++++++++++---------------- 1 file changed, 10 insertions(+), 16 deletions(-) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index c25e282d51..74e06e7604 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -6293,14 +6293,14 @@ int qemuMonitorJSONGetKVMState(qemuMonitor *mon, } -int qemuMonitorJSONGetObjectTypes(qemuMonitor *mon, - char ***types) +int +qemuMonitorJSONGetObjectTypes(qemuMonitor *mon, + char ***types) { - int ret = -1; - virJSONValue *cmd; - virJSONValue *reply = NULL; + g_autoptr(virJSONValue) cmd = NULL; + g_autoptr(virJSONValue) reply = NULL; virJSONValue *data; - char **typelist = NULL; + g_auto(GStrv) typelist = NULL; size_t n = 0; size_t i; @@ -6310,10 +6310,10 @@ int qemuMonitorJSONGetObjectTypes(qemuMonitor *mon, return -1; if (qemuMonitorJSONCommand(mon, cmd, &reply) < 0) - goto cleanup; + return -1; if (qemuMonitorJSONCheckReply(cmd, reply, VIR_JSON_TYPE_ARRAY) < 0) - goto cleanup; + return -1; data = virJSONValueObjectGetArray(reply, "return"); n = virJSONValueArraySize(data); @@ -6328,20 +6328,14 @@ int qemuMonitorJSONGetObjectTypes(qemuMonitor *mon, if (!(tmp = virJSONValueObjectGetString(child, "name"))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("qom-list-types reply data was missing 'name'")); - goto cleanup; + return -1; } typelist[i] = g_strdup(tmp); } - ret = n; *types = g_steal_pointer(&typelist); - - cleanup: - g_strfreev(typelist); - virJSONValueFree(cmd); - virJSONValueFree(reply); - return ret; + return n; } -- 2.31.1
Use g_auto(GStrv) for clearing the string list and thus remove the 'cleanup' section and 'ret' variable. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/qemumonitorjsontest.c | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index 0dce8f6ab5..5d03f83787 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -502,10 +502,8 @@ testQemuMonitorJSONGetCommands(const void *opaque) { const testGenericData *data = opaque; virDomainXMLOption *xmlopt = data->xmlopt; - int ret = -1; - char **commands = NULL; + g_auto(GStrv) commands = NULL; int ncommands = 0; - size_t i; g_autoptr(qemuMonitorTest) test = NULL; if (!(test = qemuMonitorTestNewSchema(xmlopt, data->schema))) @@ -525,16 +523,16 @@ testQemuMonitorJSONGetCommands(const void *opaque) " } " " ]" "}") < 0) - goto cleanup; + return -1; if ((ncommands = qemuMonitorGetCommands(qemuMonitorTestGetMonitor(test), &commands)) < 0) - goto cleanup; + return -1; if (ncommands != 3) { virReportError(VIR_ERR_INTERNAL_ERROR, "ncommands %d is not 3", ncommands); - goto cleanup; + return -1; } #define CHECK(i, wantname) \ @@ -543,7 +541,7 @@ testQemuMonitorJSONGetCommands(const void *opaque) virReportError(VIR_ERR_INTERNAL_ERROR, \ "name %s is not %s", \ commands[i], (wantname)); \ - goto cleanup; \ + return -1; \ } \ } while (0) @@ -552,13 +550,8 @@ testQemuMonitorJSONGetCommands(const void *opaque) CHECK(2, "quit"); #undef CHECK - ret = 0; - cleanup: - for (i = 0; i < ncommands; i++) - VIR_FREE(commands[i]); - VIR_FREE(commands); - return ret; + return 0; } -- 2.31.1
Use automatic memory freeing and remove the cleanup section.t Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- tests/qemumonitorjsontest.c | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index 5d03f83787..ab589135ba 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -560,8 +560,7 @@ testQemuMonitorJSONGetTPMModels(const void *opaque) { const testGenericData *data = opaque; virDomainXMLOption *xmlopt = data->xmlopt; - int ret = -1; - char **tpmmodels = NULL; + g_auto(GStrv) tpmmodels = NULL; int ntpmmodels = 0; g_autoptr(qemuMonitorTest) test = NULL; @@ -574,16 +573,16 @@ testQemuMonitorJSONGetTPMModels(const void *opaque) " \"passthrough\"" " ]" "}") < 0) - goto cleanup; + return -1; if ((ntpmmodels = qemuMonitorGetTPMModels(qemuMonitorTestGetMonitor(test), &tpmmodels)) < 0) - goto cleanup; + return -1; if (ntpmmodels != 1) { virReportError(VIR_ERR_INTERNAL_ERROR, "ntpmmodels %d is not 1", ntpmmodels); - goto cleanup; + return -1; } #define CHECK(i, wantname) \ @@ -592,7 +591,7 @@ testQemuMonitorJSONGetTPMModels(const void *opaque) virReportError(VIR_ERR_INTERNAL_ERROR, \ "name %s is not %s", \ tpmmodels[i], (wantname)); \ - goto cleanup; \ + return -1; \ } \ } while (0) @@ -600,11 +599,7 @@ testQemuMonitorJSONGetTPMModels(const void *opaque) #undef CHECK - ret = 0; - - cleanup: - g_strfreev(tpmmodels); - return ret; + return 0; } -- 2.31.1
All the capability getters which return a string list do in fact return a NULL-terminated list so we can use g_auto(GStrv) to free it. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_capabilities.c | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 436fe40f65..18a682f2a6 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -2564,7 +2564,7 @@ static int virQEMUCapsProbeQMPCommands(virQEMUCaps *qemuCaps, qemuMonitor *mon) { - char **commands = NULL; + g_auto(GStrv) commands = NULL; int ncommands; if ((ncommands = qemuMonitorGetCommands(mon, &commands)) < 0) @@ -2574,7 +2574,6 @@ virQEMUCapsProbeQMPCommands(virQEMUCaps *qemuCaps, G_N_ELEMENTS(virQEMUCapsCommands), virQEMUCapsCommands, ncommands, commands); - virStringListFreeCount(commands, ncommands); return 0; } @@ -2584,8 +2583,8 @@ static int virQEMUCapsProbeQMPObjectTypes(virQEMUCaps *qemuCaps, qemuMonitor *mon) { + g_auto(GStrv) values = NULL; int nvalues; - char **values; if ((nvalues = qemuMonitorGetObjectTypes(mon, &values)) < 0) return -1; @@ -2593,7 +2592,6 @@ virQEMUCapsProbeQMPObjectTypes(virQEMUCaps *qemuCaps, G_N_ELEMENTS(virQEMUCapsObjectTypes), virQEMUCapsObjectTypes, nvalues, values); - virStringListFreeCount(values, nvalues); return 0; } @@ -2863,8 +2861,6 @@ virQEMUCapsProbeQMPMachineProps(virQEMUCaps *qemuCaps, virDomainVirtType virtType, qemuMonitor *mon) { - char **values; - int nvalues; size_t i; if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_QOM_LIST_PROPERTIES)) @@ -2874,6 +2870,8 @@ virQEMUCapsProbeQMPMachineProps(virQEMUCaps *qemuCaps, virQEMUCapsObjectTypeProps props = virQEMUCapsMachineProps[i]; const char *canon = virQEMUCapsGetCanonicalMachine(qemuCaps, virtType, props.type); g_autofree char *type = NULL; + g_auto(GStrv) values = NULL; + int nvalues; if (STRNEQ(canon, "none") && !virQEMUCapsIsMachineSupported(qemuCaps, virtType, canon)) { @@ -2891,8 +2889,6 @@ virQEMUCapsProbeQMPMachineProps(virQEMUCaps *qemuCaps, props.nprops, props.props, nvalues, values); - - virStringListFreeCount(values, nvalues); } return 0; @@ -3309,7 +3305,7 @@ static int virQEMUCapsProbeQMPMigrationCapabilities(virQEMUCaps *qemuCaps, qemuMonitor *mon) { - char **caps = NULL; + g_auto(GStrv) caps = NULL; int ncaps; if ((ncaps = qemuMonitorGetMigrationCapabilities(mon, &caps)) < 0) @@ -3319,7 +3315,6 @@ virQEMUCapsProbeQMPMigrationCapabilities(virQEMUCaps *qemuCaps, G_N_ELEMENTS(virQEMUCapsMigration), virQEMUCapsMigration, ncaps, caps); - virStringListFreeCount(caps, ncaps); return 0; } -- 2.31.1
All callers pass in NULL-terminated string lists. Remove the 'nvalues' argument and fix all callers. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_capabilities.c | 36 ++++++++++++++++-------------------- 1 file changed, 16 insertions(+), 20 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 18a682f2a6..576fb6429b 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -1779,16 +1779,17 @@ static void virQEMUCapsProcessStringFlags(virQEMUCaps *qemuCaps, size_t nflags, struct virQEMUCapsStringFlags *flags, - size_t nvalues, - char *const*values) + char **values) { - size_t i, j; + size_t i; + char **value; + for (i = 0; i < nflags; i++) { if (virQEMUCapsGet(qemuCaps, flags[i].flag)) continue; - for (j = 0; j < nvalues; j++) { - if (STREQ(values[j], flags[i].value)) { + for (value = values; *value; value++) { + if (STREQ(*value, flags[i].value)) { virQEMUCapsSet(qemuCaps, flags[i].flag); break; } @@ -2565,15 +2566,14 @@ virQEMUCapsProbeQMPCommands(virQEMUCaps *qemuCaps, qemuMonitor *mon) { g_auto(GStrv) commands = NULL; - int ncommands; - if ((ncommands = qemuMonitorGetCommands(mon, &commands)) < 0) + if (qemuMonitorGetCommands(mon, &commands) < 0) return -1; virQEMUCapsProcessStringFlags(qemuCaps, G_N_ELEMENTS(virQEMUCapsCommands), virQEMUCapsCommands, - ncommands, commands); + commands); return 0; } @@ -2584,14 +2584,13 @@ virQEMUCapsProbeQMPObjectTypes(virQEMUCaps *qemuCaps, qemuMonitor *mon) { g_auto(GStrv) values = NULL; - int nvalues; - if ((nvalues = qemuMonitorGetObjectTypes(mon, &values)) < 0) + if (qemuMonitorGetObjectTypes(mon, &values) < 0) return -1; virQEMUCapsProcessStringFlags(qemuCaps, G_N_ELEMENTS(virQEMUCapsObjectTypes), virQEMUCapsObjectTypes, - nvalues, values); + values); return 0; } @@ -2645,19 +2644,18 @@ virQEMUCapsProbeQMPObjectProperties(virQEMUCaps *qemuCaps, for (i = 0; i < G_N_ELEMENTS(virQEMUCapsObjectProps); i++) { virQEMUCapsObjectTypeProps *props = virQEMUCapsObjectProps + i; g_auto(GStrv) values = NULL; - int nvalues; if (props->capsCondition >= 0 && !virQEMUCapsGet(qemuCaps, props->capsCondition)) continue; - if ((nvalues = qemuMonitorGetObjectProps(mon, props->type, &values)) < 0) + if (qemuMonitorGetObjectProps(mon, props->type, &values) < 0) return -1; virQEMUCapsProcessStringFlags(qemuCaps, props->nprops, props->props, - nvalues, values); + values); } return 0; @@ -2871,7 +2869,6 @@ virQEMUCapsProbeQMPMachineProps(virQEMUCaps *qemuCaps, const char *canon = virQEMUCapsGetCanonicalMachine(qemuCaps, virtType, props.type); g_autofree char *type = NULL; g_auto(GStrv) values = NULL; - int nvalues; if (STRNEQ(canon, "none") && !virQEMUCapsIsMachineSupported(qemuCaps, virtType, canon)) { @@ -2882,13 +2879,13 @@ virQEMUCapsProbeQMPMachineProps(virQEMUCaps *qemuCaps, * followed by the -machine suffix */ type = g_strdup_printf("%s-machine", canon); - if ((nvalues = qemuMonitorGetObjectProps(mon, type, &values)) < 0) + if (qemuMonitorGetObjectProps(mon, type, &values) < 0) return -1; virQEMUCapsProcessStringFlags(qemuCaps, props.nprops, props.props, - nvalues, values); + values); } return 0; @@ -3306,15 +3303,14 @@ virQEMUCapsProbeQMPMigrationCapabilities(virQEMUCaps *qemuCaps, qemuMonitor *mon) { g_auto(GStrv) caps = NULL; - int ncaps; - if ((ncaps = qemuMonitorGetMigrationCapabilities(mon, &caps)) < 0) + if (qemuMonitorGetMigrationCapabilities(mon, &caps) < 0) return -1; virQEMUCapsProcessStringFlags(qemuCaps, G_N_ELEMENTS(virQEMUCapsMigration), virQEMUCapsMigration, - ncaps, caps); + caps); return 0; } -- 2.31.1
This refactors multiple aspects of the function: 1) Use automatic memory freeing 2) Remove need to check element count in the returned arrays 3) Fixes questionable code linebreaks 4) Removes reuse of variables Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_capabilities.c | 33 ++++++++++++--------------------- 1 file changed, 12 insertions(+), 21 deletions(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 576fb6429b..bd7a24cf38 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -3167,36 +3167,27 @@ static int virQEMUCapsProbeQMPTPM(virQEMUCaps *qemuCaps, qemuMonitor *mon) { - int nentries; + g_auto(GStrv) models = NULL; + g_auto(GStrv) types = NULL; size_t i; - char **entries = NULL; - if ((nentries = qemuMonitorGetTPMModels(mon, &entries)) < 0) + if (qemuMonitorGetTPMModels(mon, &models) < 0) return -1; - if (nentries > 0) { - for (i = 0; i < G_N_ELEMENTS(virQEMUCapsTPMModelsToCaps); i++) { - const char *needle = virDomainTPMModelTypeToString( - virQEMUCapsTPMModelsToCaps[i].type); - if (g_strv_contains((const char **)entries, needle)) - virQEMUCapsSet(qemuCaps, - virQEMUCapsTPMModelsToCaps[i].caps); - } + for (i = 0; i < G_N_ELEMENTS(virQEMUCapsTPMModelsToCaps); i++) { + const char *needle = virDomainTPMModelTypeToString(virQEMUCapsTPMModelsToCaps[i].type); + if (g_strv_contains((const char **)models, needle)) + virQEMUCapsSet(qemuCaps, virQEMUCapsTPMModelsToCaps[i].caps); } - g_strfreev(entries); - if ((nentries = qemuMonitorGetTPMTypes(mon, &entries)) < 0) + if (qemuMonitorGetTPMTypes(mon, &types) < 0) return -1; - if (nentries > 0) { - for (i = 0; i < G_N_ELEMENTS(virQEMUCapsTPMTypesToCaps); i++) { - const char *needle = virDomainTPMBackendTypeToString( - virQEMUCapsTPMTypesToCaps[i].type); - if (g_strv_contains((const char **)entries, needle)) - virQEMUCapsSet(qemuCaps, virQEMUCapsTPMTypesToCaps[i].caps); - } + for (i = 0; i < G_N_ELEMENTS(virQEMUCapsTPMTypesToCaps); i++) { + const char *needle = virDomainTPMBackendTypeToString(virQEMUCapsTPMTypesToCaps[i].type); + if (g_strv_contains((const char **)types, needle)) + virQEMUCapsSet(qemuCaps, virQEMUCapsTPMTypesToCaps[i].caps); } - g_strfreev(entries); return 0; } -- 2.31.1
There's just one caller who cares (testQemuMonitorJSONGetTPMModels). Fix it and remove the counting of elements. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_monitor_json.c | 2 +- tests/qemumonitorjsontest.c | 10 ++++------ 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 74e06e7604..340d8a1027 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -7219,7 +7219,7 @@ qemuMonitorJSONGetStringArray(qemuMonitor *mon, if (!(*array = virJSONValueObjectGetStringArray(reply, "return"))) return -1; - return g_strv_length(*array); + return 0; } int qemuMonitorJSONGetTPMModels(qemuMonitor *mon, diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c index ab589135ba..e6746b806f 100644 --- a/tests/qemumonitorjsontest.c +++ b/tests/qemumonitorjsontest.c @@ -561,7 +561,6 @@ testQemuMonitorJSONGetTPMModels(const void *opaque) const testGenericData *data = opaque; virDomainXMLOption *xmlopt = data->xmlopt; g_auto(GStrv) tpmmodels = NULL; - int ntpmmodels = 0; g_autoptr(qemuMonitorTest) test = NULL; if (!(test = qemuMonitorTestNewSchema(xmlopt, data->schema))) @@ -575,13 +574,12 @@ testQemuMonitorJSONGetTPMModels(const void *opaque) "}") < 0) return -1; - if ((ntpmmodels = qemuMonitorGetTPMModels(qemuMonitorTestGetMonitor(test), - &tpmmodels)) < 0) + if (qemuMonitorGetTPMModels(qemuMonitorTestGetMonitor(test), &tpmmodels) < 0) return -1; - if (ntpmmodels != 1) { - virReportError(VIR_ERR_INTERNAL_ERROR, - "ntpmmodels %d is not 1", ntpmmodels); + if (g_strv_length(tpmmodels) != 1) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + "expected 1 tpm model"); return -1; } -- 2.31.1
The only caller doesn't care about the number of elements in the string list so we don't have to calculate it. Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/qemu/qemu_monitor_json.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_monitor_json.c b/src/qemu/qemu_monitor_json.c index 340d8a1027..223777739d 100644 --- a/src/qemu/qemu_monitor_json.c +++ b/src/qemu/qemu_monitor_json.c @@ -6524,7 +6524,7 @@ qemuMonitorJSONGetStringListProperty(qemuMonitor *mon, if (!(*strList = virJSONValueObjectGetStringArray(reply, "return"))) return -1; - return g_strv_length(*strList); + return 0; } -- 2.31.1
On 6/15/21 12:27 PM, Peter Krempa wrote:
Peter Krempa (13): qemuMonitorJSONParsePropsList: Refactor cleanup qemuMonitorJSONGetObjectProps: Refactor cleanup qemuMonitorJSONGetMigrationCapabilities: Refactor cleanup qemuMonitorJSONGetCommands: Refactor cleanup qemuMonitorJSONGetStringArray: Refactor cleanup qemuMonitorJSONGetObjectTypes: Refactor cleanup testQemuMonitorJSONGetCommands: Refactor cleanup testQemuMonitorJSONGetTPMModels: Refactor cleanup qemu: capabilities: Use g_auto(GStrv) instead of virStringListFreeCount virQEMUCapsProcessStringFlags: Don't require 'nvalues' virQEMUCapsProbeQMPTPM: Refactor handling of string lists qemuMonitorJSONGetStringArray: Don't return element count qemuMonitorJSONGetStringListProperty: Don't return element count
src/qemu/qemu_capabilities.c | 82 ++++++++------------- src/qemu/qemu_monitor_json.c | 138 ++++++++++++----------------------- tests/qemumonitorjsontest.c | 46 ++++-------- 3 files changed, 96 insertions(+), 170 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
participants (2)
-
Jano Tomko -
Peter Krempa