[libvirt PATCH 00/20] use g_auto for virCommand (Episode II: The Sequel)

TBD: docs/ and src/util Ján Tomko (20): openvz: refactor openvzExtractVersionInfo openvz: refactor openvzLoadDomains openvz: refactor openvzGetVEID openvz: refactor openvzDomainDefineCmd openvz: refactor openvzSetInitialConfig openvz: refactor openvzSetDiskQuota openvz: refactor openvzDomainSetNetwork openvz: refactor openvzConnectListDomains openvz: refactor openvzConnectListDefinedDomains openvz: refactor openvzDomainGetBarrierLimit openvz: refactor openvzDomainSetBarrierLimit openvz: refactor openvzGetVEStatus openvz: use g_auto in openvzDomainMigratePerform3Params openvz: refactor openvzVEGetStringParam vz: utils: use g_auto for virCommand vmware: refactor vmwareUpdateVMStatus vmware: use g_auto in vmwareLoadDomains vmware: refactor vmwareExtractVersion tests: use g_auto in testCompareXMLToConfFiles tests: storagevolxml2argvtest: do not reuse cmd src/openvz/openvz_conf.c | 62 ++++------- src/openvz/openvz_driver.c | 197 +++++++++++++-------------------- src/openvz/openvz_util.c | 23 ++-- src/vmware/vmware_conf.c | 33 ++---- src/vmware/vmware_driver.c | 19 +--- src/vz/vz_utils.c | 3 +- tests/networkxml2conftest.c | 5 +- tests/storagevolxml2argvtest.c | 4 +- 8 files changed, 125 insertions(+), 221 deletions(-) -- 2.31.1

Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/openvz/openvz_conf.c | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/src/openvz/openvz_conf.c b/src/openvz/openvz_conf.c index 4fb18c03ee..53170a720b 100644 --- a/src/openvz/openvz_conf.c +++ b/src/openvz/openvz_conf.c @@ -66,11 +66,10 @@ strtoI(const char *str) static int openvzExtractVersionInfo(const char *cmdstr, int *retversion) { - int ret = -1; unsigned long version; - char *help = NULL; + g_autofree char *help = NULL; char *tmp; - virCommand *cmd = virCommandNewArgList(cmdstr, "--help", NULL); + g_autoptr(virCommand) cmd = virCommandNewArgList(cmdstr, "--help", NULL); if (retversion) *retversion = 0; @@ -79,27 +78,21 @@ openvzExtractVersionInfo(const char *cmdstr, int *retversion) virCommandSetOutputBuffer(cmd, &help); if (virCommandRun(cmd, NULL) < 0) - goto cleanup; + return -1; tmp = help; /* expected format: vzctl version <major>.<minor>.<micro> */ if ((tmp = STRSKIP(tmp, "vzctl version ")) == NULL) - goto cleanup; + return -1; if (virParseVersionString(tmp, &version, true) < 0) - goto cleanup; + return -1; if (retversion) *retversion = version; - ret = 0; - - cleanup: - virCommandFree(cmd); - VIR_FREE(help); - - return ret; + return 0; } int openvzExtractVersion(struct openvz_driver *driver) -- 2.31.1

Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/openvz/openvz_conf.c | 32 +++++++++++--------------------- 1 file changed, 11 insertions(+), 21 deletions(-) diff --git a/src/openvz/openvz_conf.c b/src/openvz/openvz_conf.c index 53170a720b..94c3baf87f 100644 --- a/src/openvz/openvz_conf.c +++ b/src/openvz/openvz_conf.c @@ -440,10 +440,10 @@ int openvzLoadDomains(struct openvz_driver *driver) char uuidstr[VIR_UUID_STRING_BUFLEN]; virDomainObj *dom = NULL; g_autoptr(virDomainDef) def = NULL; - char *temp = NULL; - char *outbuf = NULL; + g_autofree char *temp = NULL; + g_autofree char *outbuf = NULL; char *line; - virCommand *cmd = NULL; + g_autoptr(virCommand) cmd = NULL; unsigned int vcpus = 0; if (openvzAssignUUIDs() < 0) @@ -452,7 +452,7 @@ int openvzLoadDomains(struct openvz_driver *driver) cmd = virCommandNewArgList(VZLIST, "-a", "-ovpsid,status", "-H", NULL); virCommandSetOutputBuffer(cmd, &outbuf); if (virCommandRun(cmd, NULL) < 0) - goto cleanup; + return -1; line = outbuf; while (line[0] != '\0') { @@ -462,12 +462,12 @@ int openvzLoadDomains(struct openvz_driver *driver) (line = strchr(status, '\n')) == NULL) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Failed to parse vzlist output")); - goto cleanup; + return -1; } *line++ = '\0'; if (!(def = virDomainDefNew(driver->xmlopt))) - goto cleanup; + return -1; def->virtType = VIR_DOMAIN_VIRT_OPENVZ; @@ -483,7 +483,7 @@ int openvzLoadDomains(struct openvz_driver *driver) if (ret == -1) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("UUID in config file malformed")); - goto cleanup; + return -1; } def->os.type = VIR_DOMAIN_OSTYPE_EXE; @@ -494,7 +494,7 @@ int openvzLoadDomains(struct openvz_driver *driver) virReportError(VIR_ERR_INTERNAL_ERROR, _("Could not read config for container %d"), veid); - goto cleanup; + return -1; } else if (ret > 0) { vcpus = strtoI(temp); } @@ -503,10 +503,10 @@ int openvzLoadDomains(struct openvz_driver *driver) vcpus = virHostCPUGetCount(); if (virDomainDefSetVcpusMax(def, vcpus, driver->xmlopt) < 0) - goto cleanup; + return -1; if (virDomainDefSetVcpus(def, vcpus) < 0) - goto cleanup; + return -1; /* XXX load rest of VM config data .... */ @@ -524,7 +524,7 @@ int openvzLoadDomains(struct openvz_driver *driver) driver->xmlopt, flags, NULL))) - goto cleanup; + return -1; if (STREQ(status, "stopped")) { virDomainObjSetState(dom, VIR_DOMAIN_SHUTOFF, @@ -542,17 +542,7 @@ int openvzLoadDomains(struct openvz_driver *driver) dom = NULL; } - virCommandFree(cmd); - VIR_FREE(temp); - VIR_FREE(outbuf); - return 0; - - cleanup: - virCommandFree(cmd); - VIR_FREE(temp); - VIR_FREE(outbuf); - return -1; } static int -- 2.31.1

Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/openvz/openvz_conf.c | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/src/openvz/openvz_conf.c b/src/openvz/openvz_conf.c index 94c3baf87f..d2acbc2606 100644 --- a/src/openvz/openvz_conf.c +++ b/src/openvz/openvz_conf.c @@ -978,23 +978,18 @@ static int openvzAssignUUIDs(void) int openvzGetVEID(const char *name) { - virCommand *cmd; - char *outbuf; + g_autoptr(virCommand) cmd = NULL; + g_autofree char *outbuf = NULL; char *temp; int veid; bool ok; cmd = virCommandNewArgList(VZLIST, name, "-ovpsid", "-H", NULL); virCommandSetOutputBuffer(cmd, &outbuf); - if (virCommandRun(cmd, NULL) < 0) { - virCommandFree(cmd); - VIR_FREE(outbuf); + if (virCommandRun(cmd, NULL) < 0) return -1; - } - virCommandFree(cmd); ok = virStrToLong_i(outbuf, &temp, 10, &veid) == 0 && *temp == '\n'; - VIR_FREE(outbuf); if (ok && veid >= 0) return veid; -- 2.31.1

Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/openvz/openvz_driver.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c index ce3262c0e2..efd334a235 100644 --- a/src/openvz/openvz_driver.c +++ b/src/openvz/openvz_driver.c @@ -118,15 +118,14 @@ openvzDomObjFromDomain(struct openvz_driver *driver, static virCommand * openvzDomainDefineCmd(virDomainDef *vmdef) { - virCommand *cmd = virCommandNewArgList(VZCTL, - "--quiet", - "create", - NULL); + g_autoptr(virCommand) cmd = virCommandNewArgList(VZCTL, + "--quiet", + "create", + NULL); if (vmdef == NULL) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Container is not defined")); - virCommandFree(cmd); return NULL; } @@ -137,7 +136,7 @@ openvzDomainDefineCmd(virDomainDef *vmdef) virCommandAddArgList(cmd, "--ostemplate", vmdef->fss[0]->src, NULL); } - return cmd; + return g_steal_pointer(&cmd); } -- 2.31.1

Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/openvz/openvz_driver.c | 23 ++++++++--------------- 1 file changed, 8 insertions(+), 15 deletions(-) diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c index efd334a235..c4fa83f4c6 100644 --- a/src/openvz/openvz_driver.c +++ b/src/openvz/openvz_driver.c @@ -142,14 +142,12 @@ openvzDomainDefineCmd(virDomainDef *vmdef) static int openvzSetInitialConfig(virDomainDef *vmdef) { - int ret = -1; int vpsid; - virCommand *cmd = NULL; if (vmdef->nfss > 1) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("only one filesystem supported")); - goto cleanup; + return -1; } if (vmdef->nfss == 1 && @@ -158,7 +156,7 @@ static int openvzSetInitialConfig(virDomainDef *vmdef) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("filesystem is not of type 'template' or 'mount'")); - goto cleanup; + return -1; } @@ -169,32 +167,27 @@ static int openvzSetInitialConfig(virDomainDef *vmdef) if (virStrToLong_i(vmdef->name, NULL, 10, &vpsid) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not convert domain name to VEID")); - goto cleanup; + return -1; } if (openvzCopyDefaultConfig(vpsid) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not copy default config")); - goto cleanup; + return -1; } if (openvzWriteVPSConfigParam(vpsid, "VE_PRIVATE", vmdef->fss[0]->src->path) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not set the source dir for the filesystem")); - goto cleanup; + return -1; } } else { - cmd = openvzDomainDefineCmd(vmdef); + g_autoptr(virCommand) cmd = openvzDomainDefineCmd(vmdef); if (virCommandRun(cmd, NULL) < 0) - goto cleanup; + return -1; } - ret = 0; - - cleanup: - virCommandFree(cmd); - - return ret; + return 0; } -- 2.31.1

Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/openvz/openvz_driver.c | 21 ++++++++------------- 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c index c4fa83f4c6..073de66611 100644 --- a/src/openvz/openvz_driver.c +++ b/src/openvz/openvz_driver.c @@ -196,13 +196,12 @@ openvzSetDiskQuota(virDomainDef *vmdef, virDomainFSDef *fss, bool persist) { - int ret = -1; unsigned long long sl, hl; - virCommand *cmd = virCommandNewArgList(VZCTL, - "--quiet", - "set", - vmdef->name, - NULL); + g_autoptr(virCommand) cmd = virCommandNewArgList(VZCTL, + "--quiet", + "set", + vmdef->name, + NULL); if (persist) virCommandAddArg(cmd, "--save"); @@ -220,18 +219,14 @@ openvzSetDiskQuota(virDomainDef *vmdef, } else if (fss->space_soft_limit) { virReportError(VIR_ERR_INVALID_ARG, "%s", _("Can't set soft limit without hard limit")); - goto cleanup; + return -1; } if (virCommandRun(cmd, NULL) < 0) - goto cleanup; + return -1; } - ret = 0; - cleanup: - virCommandFree(cmd); - - return ret; + return 0; } -- 2.31.1

Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/openvz/openvz_driver.c | 21 +++++++-------------- 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c index 073de66611..cffdb20898 100644 --- a/src/openvz/openvz_driver.c +++ b/src/openvz/openvz_driver.c @@ -661,13 +661,12 @@ openvzDomainSetNetwork(virConnectPtr conn, const char *vpsid, virDomainNetDef *net, virBuffer *configBuf) { - int rc = -1; char macaddr[VIR_MAC_STRING_BUFLEN]; virMacAddr host_mac; char host_macaddr[VIR_MAC_STRING_BUFLEN]; struct openvz_driver *driver = conn->privateData; - virCommand *cmd = NULL; - char *guest_ifname = NULL; + g_autoptr(virCommand) cmd = NULL; + g_autofree char *guest_ifname = NULL; if (net == NULL) return 0; @@ -703,7 +702,7 @@ openvzDomainSetNetwork(virConnectPtr conn, const char *vpsid, if (guest_ifname == NULL) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not generate eth name for container")); - goto cleanup; + return -1; } } @@ -714,7 +713,7 @@ openvzDomainSetNetwork(virConnectPtr conn, const char *vpsid, if (net->ifname == NULL) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Could not generate veth name")); - goto cleanup; + return -1; } } @@ -744,23 +743,17 @@ openvzDomainSetNetwork(virConnectPtr conn, const char *vpsid, /* --ipadd ip */ for (i = 0; i < net->guestIP.nips; i++) { - char *ipStr = virSocketAddrFormat(&net->guestIP.ips[i]->address); + g_autofree char *ipStr = virSocketAddrFormat(&net->guestIP.ips[i]->address); if (!ipStr) - goto cleanup; + return -1; virCommandAddArgList(cmd, "--ipadd", ipStr, NULL); - VIR_FREE(ipStr); } } /* TODO: processing NAT and physical device */ virCommandAddArg(cmd, "--save"); - rc = virCommandRun(cmd, NULL); - - cleanup: - virCommandFree(cmd); - VIR_FREE(guest_ifname); - return rc; + return virCommandRun(cmd, NULL); } -- 2.31.1

Use g_auto where possible, reduce scope of some variables and remove pointless ret and rc variables. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/openvz/openvz_driver.c | 31 ++++++++++++------------------- 1 file changed, 12 insertions(+), 19 deletions(-) diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c index cffdb20898..fa5ded54ec 100644 --- a/src/openvz/openvz_driver.c +++ b/src/openvz/openvz_driver.c @@ -1334,44 +1334,37 @@ static int openvzConnectListDomains(virConnectPtr conn G_GNUC_UNUSED, int *ids, int nids) { int got = 0; - int veid; - int outfd = -1; - int rc = -1; - int ret; - char buf[32]; - char *endptr; - virCommand *cmd = virCommandNewArgList(VZLIST, "-ovpsid", "-H", NULL); + VIR_AUTOCLOSE outfd = -1; + g_autoptr(virCommand) cmd = virCommandNewArgList(VZLIST, "-ovpsid", "-H", NULL); virCommandSetOutputFD(cmd, &outfd); if (virCommandRunAsync(cmd, NULL) < 0) - goto cleanup; + return -1; while (got < nids) { - ret = openvz_readline(outfd, buf, 32); - if (!ret) + char *endptr; + char buf[32]; + int veid; + + if (openvz_readline(outfd, buf, 32) == 0) break; if (virStrToLong_i(buf, &endptr, 10, &veid) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Could not parse VPS ID %s"), buf); continue; } - ids[got] = veid; - got ++; + ids[got++] = veid; } if (virCommandWait(cmd, NULL) < 0) - goto cleanup; + return -1; if (VIR_CLOSE(outfd) < 0) { virReportSystemError(errno, "%s", _("failed to close file")); - goto cleanup; + return -1; } - rc = got; - cleanup: - VIR_FORCE_CLOSE(outfd); - virCommandFree(cmd); - return rc; + return got; } static int openvzConnectNumOfDomains(virConnectPtr conn) -- 2.31.1

Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/openvz/openvz_driver.c | 38 ++++++++++++++++++-------------------- 1 file changed, 18 insertions(+), 20 deletions(-) diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c index fa5ded54ec..f1ee540a8b 100644 --- a/src/openvz/openvz_driver.c +++ b/src/openvz/openvz_driver.c @@ -1382,22 +1382,23 @@ static int openvzConnectNumOfDomains(virConnectPtr conn) static int openvzConnectListDefinedDomains(virConnectPtr conn G_GNUC_UNUSED, char **const names, int nnames) { int got = 0; - int veid, outfd = -1, ret; - int rc = -1; - char vpsname[32]; - char buf[32]; - char *endptr; - virCommand *cmd = virCommandNewArgList(VZLIST, - "-ovpsid", "-H", "-S", NULL); + VIR_AUTOCLOSE outfd = -1; + int ret = -1; + g_autoptr(virCommand) cmd = virCommandNewArgList(VZLIST, + "-ovpsid", "-H", "-S", NULL); /* the -S options lists only stopped domains */ virCommandSetOutputFD(cmd, &outfd); if (virCommandRunAsync(cmd, NULL) < 0) - goto out; + goto cleanup; while (got < nnames) { - ret = openvz_readline(outfd, buf, 32); - if (!ret) + char vpsname[32]; + char buf[32]; + char *endptr; + int veid; + + if (openvz_readline(outfd, buf, 32) == 0) break; if (virStrToLong_i(buf, &endptr, 10, &veid) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -1405,27 +1406,24 @@ static int openvzConnectListDefinedDomains(virConnectPtr conn G_GNUC_UNUSED, continue; } g_snprintf(vpsname, sizeof(vpsname), "%d", veid); - names[got] = g_strdup(vpsname); - got ++; + names[got++] = g_strdup(vpsname); } if (virCommandWait(cmd, NULL) < 0) - goto out; + goto cleanup; if (VIR_CLOSE(outfd) < 0) { virReportSystemError(errno, "%s", _("failed to close file")); - goto out; + goto cleanup; } - rc = got; - out: - VIR_FORCE_CLOSE(outfd); - virCommandFree(cmd); - if (rc < 0) { + ret = got; + cleanup: + if (ret < 0) { for (; got >= 0; got--) VIR_FREE(names[got]); } - return rc; + return ret; } static int openvzGetProcessInfo(unsigned long long *cpuTime, int vpsid) -- 2.31.1

Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/openvz/openvz_driver.c | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c index f1ee540a8b..345c31304a 100644 --- a/src/openvz/openvz_driver.c +++ b/src/openvz/openvz_driver.c @@ -1515,37 +1515,33 @@ openvzDomainGetBarrierLimit(virDomainPtr domain, unsigned long long *barrier, unsigned long long *limit) { - int ret = -1; - char *endp, *output = NULL; + char *endp; + g_autofree char *output = NULL; const char *tmp; - virCommand *cmd = virCommandNewArgList(VZLIST, "--no-header", NULL); + g_autoptr(virCommand) cmd = virCommandNewArgList(VZLIST, "--no-header", NULL); virCommandSetOutputBuffer(cmd, &output); virCommandAddArgFormat(cmd, "-o%s.b,%s.l", param, param); virCommandAddArg(cmd, domain->name); if (virCommandRun(cmd, NULL) < 0) - goto cleanup; + return -1; tmp = output; virSkipSpaces(&tmp); if (virStrToLong_ull(tmp, &endp, 10, barrier) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Can't parse limit from vzlist output '%s'"), output); - goto cleanup; + return -1; } tmp = endp; virSkipSpaces(&tmp); if (virStrToLong_ull(tmp, &endp, 10, limit) < 0) { virReportError(VIR_ERR_INTERNAL_ERROR, _("Can't parse barrier from vzlist output '%s'"), output); - goto cleanup; + return -1; } - ret = 0; - cleanup: - VIR_FREE(output); - virCommandFree(cmd); - return ret; + return 0; } -- 2.31.1

Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/openvz/openvz_driver.c | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c index 345c31304a..3cd655e5a7 100644 --- a/src/openvz/openvz_driver.c +++ b/src/openvz/openvz_driver.c @@ -1551,28 +1551,22 @@ openvzDomainSetBarrierLimit(virDomainPtr domain, unsigned long long barrier, unsigned long long limit) { - int ret = -1; - virCommand *cmd = virCommandNewArgList(VZCTL, "--quiet", "set", NULL); + g_autoptr(virCommand) cmd = virCommandNewArgList(VZCTL, "--quiet", "set", NULL); /* LONG_MAX indicates unlimited so reject larger values */ if (barrier > LONG_MAX || limit > LONG_MAX) { virReportError(VIR_ERR_OPERATION_FAILED, _("Failed to set %s for %s: value too large"), param, domain->name); - goto cleanup; + return -1; } virCommandAddArg(cmd, domain->name); virCommandAddArgFormat(cmd, "--%s", param); virCommandAddArgFormat(cmd, "%llu:%llu", barrier, limit); virCommandAddArg(cmd, "--save"); - if (virCommandRun(cmd, NULL) < 0) - goto cleanup; - ret = 0; - cleanup: - virCommandFree(cmd); - return ret; + return virCommandRun(cmd, NULL); } -- 2.31.1

Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/openvz/openvz_driver.c | 16 +++++----------- 1 file changed, 5 insertions(+), 11 deletions(-) diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c index 3cd655e5a7..d4c65dbc26 100644 --- a/src/openvz/openvz_driver.c +++ b/src/openvz/openvz_driver.c @@ -1697,21 +1697,20 @@ openvzDomainSetMemoryParameters(virDomainPtr domain, static int openvzGetVEStatus(virDomainObj *vm, int *status, int *reason) { - virCommand *cmd; - char *outbuf; + g_autoptr(virCommand) cmd = NULL; + g_autofree char *outbuf = NULL; char *line; int state; - int ret = -1; cmd = virCommandNewArgList(VZLIST, vm->def->name, "-ostatus", "-H", NULL); virCommandSetOutputBuffer(cmd, &outbuf); if (virCommandRun(cmd, NULL) < 0) - goto cleanup; + return -1; if ((line = strchr(outbuf, '\n')) == NULL) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Failed to parse vzlist output")); - goto cleanup; + return -1; } *line++ = '\0'; @@ -1728,12 +1727,7 @@ openvzGetVEStatus(virDomainObj *vm, int *status, int *reason) *status = VIR_DOMAIN_SHUTOFF; } - ret = 0; - - cleanup: - virCommandFree(cmd); - VIR_FREE(outbuf); - return ret; + return 0; } static int -- 2.31.1

Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/openvz/openvz_driver.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/openvz/openvz_driver.c b/src/openvz/openvz_driver.c index d4c65dbc26..b2589059c8 100644 --- a/src/openvz/openvz_driver.c +++ b/src/openvz/openvz_driver.c @@ -2113,8 +2113,8 @@ openvzDomainMigratePerform3Params(virDomainPtr domain, struct openvz_driver *driver = domain->conn->privateData; virDomainObj *vm = NULL; const char *uri_str = NULL; - virURI *uri = NULL; - virCommand *cmd = NULL; + g_autoptr(virURI) uri = NULL; + g_autoptr(virCommand) cmd = NULL; int ret = -1; virCheckFlags(OPENVZ_MIGRATION_FLAGS, -1); @@ -2146,8 +2146,6 @@ openvzDomainMigratePerform3Params(virDomainPtr domain, ret = 0; cleanup: - virCommandFree(cmd); - virURIFree(uri); virDomainObjEndAPI(&vm); return ret; } -- 2.31.1

Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/openvz/openvz_util.c | 23 +++++++++-------------- 1 file changed, 9 insertions(+), 14 deletions(-) diff --git a/src/openvz/openvz_util.c b/src/openvz/openvz_util.c index d14254c231..9419ce122c 100644 --- a/src/openvz/openvz_util.c +++ b/src/openvz/openvz_util.c @@ -58,27 +58,22 @@ char* openvzVEGetStringParam(virDomainPtr domain, const char* param) { int len; - char *output = NULL; + g_autofree char *output = NULL; - virCommand *cmd = virCommandNewArgList(VZLIST, - "-o", - param, - domain->name, - "-H", NULL); + g_autoptr(virCommand) cmd = virCommandNewArgList(VZLIST, + "-o", + param, + domain->name, + "-H", NULL); virCommandSetOutputBuffer(cmd, &output); - if (virCommandRun(cmd, NULL) < 0) { - VIR_FREE(output); - /* virCommandRun sets the virError */ - goto cleanup; - } + if (virCommandRun(cmd, NULL) < 0) + return NULL; /* delete trailing newline */ len = strlen(output); if (len && output[len - 1] == '\n') output[len - 1] = '\0'; - cleanup: - virCommandFree(cmd); - return output; + return g_steal_pointer(&output); } -- 2.31.1

Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/vz/vz_utils.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/vz/vz_utils.c b/src/vz/vz_utils.c index 8fed875281..90e26091bb 100644 --- a/src/vz/vz_utils.c +++ b/src/vz/vz_utils.c @@ -92,7 +92,7 @@ vzDomObjFromDomain(virDomainPtr domain) static int vzDoCmdRun(char **outbuf, const char *binary, va_list list) { - virCommand *cmd = virCommandNewVAList(binary, list); + g_autoptr(virCommand) cmd = virCommandNewVAList(binary, list); int ret = -1; if (outbuf) @@ -104,7 +104,6 @@ vzDoCmdRun(char **outbuf, const char *binary, va_list list) ret = 0; cleanup: - virCommandFree(cmd); if (ret && outbuf) VIR_FREE(*outbuf); return ret; -- 2.31.1

Use g_auto and get rid of the cleanup label, as well as the ret variable. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/vmware/vmware_driver.c | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/src/vmware/vmware_driver.c b/src/vmware/vmware_driver.c index 8f107dd529..998ecdb546 100644 --- a/src/vmware/vmware_driver.c +++ b/src/vmware/vmware_driver.c @@ -284,27 +284,26 @@ vmwareConnectGetVersion(virConnectPtr conn, unsigned long *version) static int vmwareUpdateVMStatus(struct vmware_driver *driver, virDomainObj *vm) { - virCommand *cmd; - char *outbuf = NULL; - char *vmxAbsolutePath = NULL; + g_autoptr(virCommand) cmd = NULL; + g_autofree char *outbuf = NULL; + g_autofree char *vmxAbsolutePath = NULL; char *parsedVmxPath = NULL; char *str; char *saveptr = NULL; bool found = false; int oldState = virDomainObjGetState(vm, NULL); int newState; - int ret = -1; cmd = virCommandNewArgList(driver->vmrun, "-T", vmwareDriverTypeToString(driver->type), "list", NULL); virCommandSetOutputBuffer(cmd, &outbuf); if (virCommandRun(cmd, NULL) < 0) - goto cleanup; + return -1; if (virFileResolveAllLinks(((vmwareDomainPtr) vm->privateData)->vmxPath, &vmxAbsolutePath) < 0) - goto cleanup; + return -1; for (str = outbuf; (parsedVmxPath = strtok_r(str, "\n", &saveptr)) != NULL; str = NULL) { @@ -331,13 +330,7 @@ vmwareUpdateVMStatus(struct vmware_driver *driver, virDomainObj *vm) virDomainObjSetState(vm, newState, 0); - ret = 0; - - cleanup: - virCommandFree(cmd); - VIR_FREE(outbuf); - VIR_FREE(vmxAbsolutePath); - return ret; + return 0; } static int -- 2.31.1

Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/vmware/vmware_conf.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/vmware/vmware_conf.c b/src/vmware/vmware_conf.c index 3f6b51ac44..17e67ea6d2 100644 --- a/src/vmware/vmware_conf.c +++ b/src/vmware/vmware_conf.c @@ -118,14 +118,14 @@ vmwareLoadDomains(struct vmware_driver *driver) { virDomainObj *vm = NULL; char *vmxPath = NULL; - char *vmx = NULL; + g_autofree char *vmx = NULL; vmwareDomainPtr pDomain; int ret = -1; virVMXContext ctx; - char *outbuf = NULL; + g_autofree char *outbuf = NULL; char *str; char *saveptr = NULL; - virCommand *cmd; + g_autoptr(virCommand) cmd = NULL; ctx.parseFileName = vmwareParseVMXFileName; ctx.formatFileName = NULL; @@ -178,9 +178,6 @@ vmwareLoadDomains(struct vmware_driver *driver) ret = 0; cleanup: - virCommandFree(cmd); - VIR_FREE(outbuf); - VIR_FREE(vmx); virObjectUnref(vm); return ret; } -- 2.31.1

Use g_auto for cleanup and remove the cleanup label. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/vmware/vmware_conf.c | 24 ++++++++---------------- 1 file changed, 8 insertions(+), 16 deletions(-) diff --git a/src/vmware/vmware_conf.c b/src/vmware/vmware_conf.c index 17e67ea6d2..c88f11fcab 100644 --- a/src/vmware/vmware_conf.c +++ b/src/vmware/vmware_conf.c @@ -242,11 +242,10 @@ vmwareParseVersionStr(int type, const char *verbuf, unsigned long *version) int vmwareExtractVersion(struct vmware_driver *driver) { - int ret = -1; - virCommand *cmd = NULL; - char * outbuf = NULL; - char *bin = NULL; - char *vmwarePath = NULL; + g_autoptr(virCommand) cmd = NULL; + g_autofree char *outbuf = NULL; + g_autofree char *bin = NULL; + g_autofree char *vmwarePath = NULL; vmwarePath = g_path_get_dirname(driver->vmrun); @@ -266,7 +265,7 @@ vmwareExtractVersion(struct vmware_driver *driver) default: virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("invalid driver type for version detection")); - goto cleanup; + return -1; } cmd = virCommandNewArgList(bin, "-v", NULL); @@ -274,19 +273,12 @@ vmwareExtractVersion(struct vmware_driver *driver) virCommandSetErrorBuffer(cmd, &outbuf); if (virCommandRun(cmd, NULL) < 0) - goto cleanup; + return -1; if (vmwareParseVersionStr(driver->type, outbuf, &driver->version) < 0) - goto cleanup; + return -1; - ret = 0; - - cleanup: - virCommandFree(cmd); - VIR_FREE(outbuf); - VIR_FREE(bin); - VIR_FREE(vmwarePath); - return ret; + return 0; } int -- 2.31.1

Use g_auto for dnsmasq context and remove the cmd variable. It was unused since its introduction in: commit 8b32c80df089a3612a0448c1a92abc2071d6b6a9 Signed-off-by: Ján Tomko <jtomko@redhat.com> --- tests/networkxml2conftest.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/tests/networkxml2conftest.c b/tests/networkxml2conftest.c index b76d72793a..7177505918 100644 --- a/tests/networkxml2conftest.c +++ b/tests/networkxml2conftest.c @@ -24,9 +24,8 @@ testCompareXMLToConfFiles(const char *inxml, const char *outconf, int ret = -1; virNetworkDef *def = NULL; virNetworkObj *obj = NULL; - virCommand *cmd = NULL; g_autofree char *pidfile = NULL; - dnsmasqContext *dctx = NULL; + g_autoptr(dnsmasqContext) dctx = NULL; g_autoptr(virNetworkXMLOption) xmlopt = NULL; if (!(xmlopt = networkDnsmasqCreateXMLConf())) @@ -82,9 +81,7 @@ testCompareXMLToConfFiles(const char *inxml, const char *outconf, fail: VIR_FREE(confactual); - virCommandFree(cmd); virNetworkObjEndAPI(&obj); - dnsmasqContextFree(dctx); return ret; } -- 2.31.1

Reduce the scope of the variable to avoid mixing automatic and manual cleanup. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- tests/storagevolxml2argvtest.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/storagevolxml2argvtest.c b/tests/storagevolxml2argvtest.c index 9597509f00..94e2de1730 100644 --- a/tests/storagevolxml2argvtest.c +++ b/tests/storagevolxml2argvtest.c @@ -50,7 +50,6 @@ testCompareXMLToArgvFiles(bool shouldFail, g_autoptr(virStorageVolDef) vol = NULL; g_autoptr(virStorageVolDef) inputvol = NULL; g_autoptr(virStoragePoolDef) inputpool = NULL; - g_autoptr(virCommand) cmd = NULL; if (!(def = virStoragePoolDefParseFile(poolxml))) goto cleanup; @@ -88,7 +87,8 @@ testCompareXMLToArgvFiles(bool shouldFail, convertStep = VIR_STORAGE_VOL_ENCRYPT_CREATE; do { - virCommandFree(cmd); + g_autoptr(virCommand) cmd = NULL; + cmd = virStorageBackendCreateQemuImgCmdFromVol(obj, vol, inputvol, flags, create_tool, -- 2.31.1

On 12/13/21 19:42, Ján Tomko wrote:
TBD: docs/ and src/util
Ján Tomko (20): openvz: refactor openvzExtractVersionInfo openvz: refactor openvzLoadDomains openvz: refactor openvzGetVEID openvz: refactor openvzDomainDefineCmd openvz: refactor openvzSetInitialConfig openvz: refactor openvzSetDiskQuota openvz: refactor openvzDomainSetNetwork openvz: refactor openvzConnectListDomains openvz: refactor openvzConnectListDefinedDomains openvz: refactor openvzDomainGetBarrierLimit openvz: refactor openvzDomainSetBarrierLimit openvz: refactor openvzGetVEStatus openvz: use g_auto in openvzDomainMigratePerform3Params openvz: refactor openvzVEGetStringParam vz: utils: use g_auto for virCommand vmware: refactor vmwareUpdateVMStatus vmware: use g_auto in vmwareLoadDomains vmware: refactor vmwareExtractVersion tests: use g_auto in testCompareXMLToConfFiles tests: storagevolxml2argvtest: do not reuse cmd
src/openvz/openvz_conf.c | 62 ++++------- src/openvz/openvz_driver.c | 197 +++++++++++++-------------------- src/openvz/openvz_util.c | 23 ++-- src/vmware/vmware_conf.c | 33 ++---- src/vmware/vmware_driver.c | 19 +--- src/vz/vz_utils.c | 3 +- tests/networkxml2conftest.c | 5 +- tests/storagevolxml2argvtest.c | 4 +- 8 files changed, 125 insertions(+), 221 deletions(-)
Reviewed-by: Michal Privoznik <mprivozn@redhat.com> Michal
participants (2)
-
Ján Tomko
-
Michal Prívozník