Re: [libvirt] [PATCH 1/6] conf: make hostdev info a separate object
On 02/20/2012 10:10 AM, Laine Stump wrote:
In order to allow for a virDomainHostdevDef that uses the
virDomainDeviceInfo of a "higher level" device (such as a
virDomainNetDef), this patch changes the virDomainDeviceInfo in the
HostdevDef into a virDomainDeviceInfoPtr. Rather than adding checks
all over the code to check for a null info, we just guarantee that it
is always valid. The new function virDomainHostdevDefAlloc() allocates
a virDomainDeviceInfo and plugs it in, and virDomainHostdevDefFree()
makes sure it is freed.
There were 4 places allocating virDomainHostdevDefs, all of them
parsers of one sort or another, and those have all had their
VIR_ALLOC(hostdev) changed to virDomainHostdevDefAlloc(). Other than
that, and the new functions, all the rest of the changes are just
mechanical removals of "&" or changing "." to "->".
+virDomainHostdevDefPtr virDomainHostdevDefAlloc(void)
+{
+ virDomainHostdevDefPtr def = NULL;
+
+ if (VIR_ALLOC(def) < 0) {
+ virReportOOMError();
+ return NULL;
+ }
+ if (VIR_ALLOC(def->info) < 0) {
+ VIR_FREE(def);
+ return NULL;
Missing virReportOOMError() on this path.
@@ -6527,15 +6527,17 @@ cleanup:
static virDomainHostdevDefPtr
qemuParseCommandLinePCI(const char *val)
{
- virDomainHostdevDefPtr def = NULL;
int bus = 0, slot = 0, func = 0;
const char *start;
char *end;
+ virDomainHostdevDefPtr def = virDomainHostdevDefAlloc();
+
+ if (!def)
+ goto cleanup;
if (!STRPREFIX(val, "host=")) {
qemuReportError(VIR_ERR_INTERNAL_ERROR,
_("unknown PCI device syntax '%s'"), val);
- VIR_FREE(def);
goto cleanup;
}
Incomplete conversion - you nuked this VIR_FREE(def), but not the others
on the remaining error paths, nor did you fix the cleanup label to call
the correct new function to free things now that HostdevDef contains an
embedded pointer. [And I have no idea why the VIR_FREE(def) was there
in the first place, given that pre-patch, def was initialized as NULL,
making the VIR_FREE a no-op]
@@ -6584,11 +6581,14 @@ cleanup:
static virDomainHostdevDefPtr
qemuParseCommandLineUSB(const char *val)
{
- virDomainHostdevDefPtr def = NULL;
+ virDomainHostdevDefPtr def = virDomainHostdevDefAlloc();
int first = 0, second = 0;
const char *start;
char *end;
+ if (!def)
+ goto cleanup;
+
if (!STRPREFIX(val, "host:")) {
qemuReportError(VIR_ERR_INTERNAL_ERROR,
_("unknown USB device syntax '%s'"), val);
Same problem here - you've left a (now-unsafe) VIR_FREE(def) in all the
error paths, instead of making cleanup: call a common function that
properly frees both def and its contents.
Overall, the idea looks reasonable, but you'll need a v2 to fix the
memory issues in qemu_command.c.
--
Eric Blake eblake(a)redhat.com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachments:
- signature.asc (application/pgp-signature — 620 bytes)