2010/12/3 Eric Blake <ebl...@redhat.com>: > popen must be matched with pclose (not fclose), or it will leak > resources. Furthermore, it is a lousy interface when it comes > to signal handling. We're much better off using our decent command > wrapper. > > * src/openvz/openvz_conf.c (openvzLoadDomains, openvzGetVEID): > Replace popen with virCommand usage. > --- > src/openvz/openvz_conf.c | 54 +++++++++++++++++++++++---------------------- > 1 files changed, 28 insertions(+), 26 deletions(-)
> int openvzLoadDomains(struct openvz_driver *driver) { > - FILE *fp; > int veid, ret; > char status[16]; > char uuidstr[VIR_UUID_STRING_BUFLEN]; > virDomainObjPtr dom = NULL; > char temp[50]; > + char *outbuf = NULL; > + char *line; > + virCommandPtr cmd = NULL; > > if (openvzAssignUUIDs() < 0) > return -1; > > - if ((fp = popen(VZLIST " -a -ovpsid,status -H 2>/dev/null", "r")) == > NULL) { > - openvzError(VIR_ERR_INTERNAL_ERROR, "%s", _("popen failed")); > - return -1; > - } > - > - while (!feof(fp)) { > - if (fscanf(fp, "%d %s\n", &veid, status) != 2) { > - if (feof(fp)) > - break; > + cmd = virCommandNewArgList(VZLIST, "-a", "-ovpsid,status", "-H", NULL); > + virCommandSetOutputBuffer(cmd, &outbuf); > + if (virCommandRun(cmd, NULL) < 0) > + goto cleanup; > > + line = *outbuf ? outbuf : NULL; Is outbuf guaranteed to be non-NULL when virCommandRun succeeds? Otherwise we have a potential NULL dereference here. > + while (line) { > + if (sscanf(line, "%d %s\n", &veid, status) != 2) { > openvzError(VIR_ERR_INTERNAL_ERROR, "%s", > _("Failed to parse vzlist output")); > goto cleanup; > @@ -978,27 +985,22 @@ static int openvzAssignUUIDs(void) > */ > > int openvzGetVEID(const char *name) { > - char *cmd; > + virCommandPtr cmd; > + char *outbuf; > int veid; > - FILE *fp; > bool ok; > > - if (virAsprintf(&cmd, "%s %s -ovpsid -H", VZLIST, name) < 0) { > - openvzError(VIR_ERR_INTERNAL_ERROR, "%s", > - _("virAsprintf failed")); > + cmd = virCommandNewArgList(VZLIST, name, "-ovpsid", "-H", NULL); > + virCommandSetOutputBuffer(cmd, &outbuf); > + if (virCommandRun(cmd, NULL) < 0) { > + virCommandFree(cmd); Is outbuf guaranteed to be unallocated when virCommandRun fails? Otherwise a VIR_FREE(outbuf) is missing here. > return -1; > } > > - fp = popen(cmd, "r"); > - VIR_FREE(cmd); > - > - if (fp == NULL) { > - openvzError(VIR_ERR_INTERNAL_ERROR, "%s", _("popen failed")); > - return -1; > - } > + virCommandFree(cmd); > + ok = sscanf(outbuf, "%d\n", &veid) == 1; Same question here about outbuf being guaranteed to be non-NULL when virCommandRun succeeds as in openvzLoadDomains. If not then sscanf is called with NULL and that'll probably segfault. > + VIR_FREE(outbuf); > > - ok = fscanf(fp, "%d\n", &veid ) == 1; > - VIR_FORCE_FCLOSE(fp); > if (ok && veid >= 0) > return veid; Matthias -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list