On 07/10/2012 02:46 PM, Guido Günther wrote:
+char* +openvzVEGetStringParam(virDomainPtr domain, const char* param) +{ + int status, len; + char *output = NULL; + + virCommandPtr cmd = virCommandNewArgList(VZLIST, + "-o", + param, + domain->name, + "-H" , NULL); + + virCommandSetOutputBuffer(cmd, &output); + if (virCommandRun(cmd, &status)) {
This should check for virCommandRun() < 0. Also, virCommandRun() does not guarantee that 'status' is populated unless it returns 0; if it returns -1, you may be left with outputting uninitilized data (maybe we could argue that it is a bug in virCommandRun that should be fixed...). Outputting the raw exit status isn't very handy; I think it would be simpler to just pass NULL for the second argument (which lets virCommandRun issue a reasonable error message about any failure to run the command, including the entire command line attempted, which already includes param and domain->name).
+ openvzError(VIR_ERR_OPERATION_FAILED, + _("Failed to get %s for %s: %d"), param, domain->name, + status); + VIR_FREE(output); + } + len = strlen(output);
Crash-tastic! If the command fails, then you end up calling strlen(NULL). Are you missing a goto or early return?
+ /* delete trailing newline */ + if (len) + output[len-1] = '\0';
Shouldn't you at least check that output[len-1] is '\n' before nuking it? Also, our style prefers spaces on either side of '-'. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org