[libvirt] [PATCH 00/16] some more vboxDump cleanups
Inspired-by: Laine Stump <laine@laine.org> Day-of-the-week: Friday <6> Ján Tomko (16): vboxDumpSharedFolders: rename non-standard label vboxDumpSharedFolders: remove pointless comment vboxDumpSharedFolders: return a value vboxDumpNetwork: add temp variable for current network vboxDumpNetwork: rename to vboxDumpNetworks vboxDumpNetwork: re-introduce this function vboxDumpNetworks: reduce indentation level vboxDumpNetwork: allocate the network too vboxDumpNetworks: delete pointless comment vboxDumpNetworks: do not allocate def->nets upfront vboxDumpNetwork: use virMacAddrParseHex vboxDumpNetwork: Use a single utf16 variable vboxDumpNetwork: Use a single utf8 temp variable vboxDumpNetwork: use a switch for attachmentType vboxDumpNetwork: use VIR_STEAL_PTR instead of VIR_STRDUP vboxDumpNetwork: use switch for adapterType src/vbox/vbox_common.c | 243 ++++++++++++++++++++++++------------------------- 1 file changed, 120 insertions(+), 123 deletions(-) -- 2.13.6
s/sharedFoldersCleanup/cleanup/ Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/vbox/vbox_common.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c index 07f430878..e1629b07f 100644 --- a/src/vbox/vbox_common.c +++ b/src/vbox/vbox_common.c @@ -3640,10 +3640,10 @@ vboxDumpSharedFolders(virDomainDefPtr def, vboxDriverPtr data, IMachine *machine gVBoxAPI.UArray.handleMachineGetSharedFolders(machine)); if (sharedFolders.count <= 0) - goto sharedFoldersCleanup; + goto cleanup; if (VIR_ALLOC_N(def->fss, sharedFolders.count) < 0) - goto sharedFoldersCleanup; + goto cleanup; for (i = 0; i < sharedFolders.count; i++) { ISharedFolder *sharedFolder = sharedFolders.items[i]; @@ -3654,7 +3654,7 @@ vboxDumpSharedFolders(virDomainDefPtr def, vboxDriverPtr data, IMachine *machine PRBool writable = PR_FALSE; if (VIR_ALLOC(def->fss[i]) < 0) - goto sharedFoldersCleanup; + goto cleanup; def->fss[i]->type = VIR_DOMAIN_FS_TYPE_MOUNT; @@ -3663,7 +3663,7 @@ vboxDumpSharedFolders(virDomainDefPtr def, vboxDriverPtr data, IMachine *machine if (VIR_STRDUP(def->fss[i]->src->path, hostPath) < 0) { VBOX_UTF8_FREE(hostPath); VBOX_UTF16_FREE(hostPathUtf16); - goto sharedFoldersCleanup; + goto cleanup; } VBOX_UTF8_FREE(hostPath); VBOX_UTF16_FREE(hostPathUtf16); @@ -3673,7 +3673,7 @@ vboxDumpSharedFolders(virDomainDefPtr def, vboxDriverPtr data, IMachine *machine if (VIR_STRDUP(def->fss[i]->dst, name) < 0) { VBOX_UTF8_FREE(name); VBOX_UTF16_FREE(nameUtf16); - goto sharedFoldersCleanup; + goto cleanup; } VBOX_UTF8_FREE(name); VBOX_UTF16_FREE(nameUtf16); @@ -3684,7 +3684,7 @@ vboxDumpSharedFolders(virDomainDefPtr def, vboxDriverPtr data, IMachine *machine ++def->nfss; } - sharedFoldersCleanup: + cleanup: gVBoxAPI.UArray.vboxArrayRelease(&sharedFolders); } -- 2.13.6
Now that the functions are separate, we no longer need comment separators. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/vbox/vbox_common.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c index e1629b07f..afd00a91a 100644 --- a/src/vbox/vbox_common.c +++ b/src/vbox/vbox_common.c @@ -3630,7 +3630,6 @@ vboxDumpDisplay(virDomainDefPtr def, vboxDriverPtr data, IMachine *machine) static void vboxDumpSharedFolders(virDomainDefPtr def, vboxDriverPtr data, IMachine *machine) { - /* shared folders */ vboxArray sharedFolders = VBOX_ARRAY_INITIALIZER; size_t i = 0; -- 2.13.6
The allocation errors in this function are already handled by jumping to a cleanup label. Change the return type from void to int and return -1 on error. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/vbox/vbox_common.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c index afd00a91a..cc7772f25 100644 --- a/src/vbox/vbox_common.c +++ b/src/vbox/vbox_common.c @@ -3627,19 +3627,23 @@ vboxDumpDisplay(virDomainDefPtr def, vboxDriverPtr data, IMachine *machine) return ret; } -static void +static int vboxDumpSharedFolders(virDomainDefPtr def, vboxDriverPtr data, IMachine *machine) { vboxArray sharedFolders = VBOX_ARRAY_INITIALIZER; size_t i = 0; + int ret = -1; def->nfss = 0; gVBoxAPI.UArray.vboxArrayGet(&sharedFolders, machine, gVBoxAPI.UArray.handleMachineGetSharedFolders(machine)); - if (sharedFolders.count <= 0) + if (sharedFolders.count <= 0) { + if (sharedFolders.count == 0) + ret = 0; goto cleanup; + } if (VIR_ALLOC_N(def->fss, sharedFolders.count) < 0) goto cleanup; @@ -3683,8 +3687,11 @@ vboxDumpSharedFolders(virDomainDefPtr def, vboxDriverPtr data, IMachine *machine ++def->nfss; } + ret = 0; + cleanup: gVBoxAPI.UArray.vboxArrayRelease(&sharedFolders); + return ret; } static void @@ -4179,7 +4186,8 @@ static char *vboxDomainGetXMLDesc(virDomainPtr dom, unsigned int flags) if (vboxDumpDisks(def, data, machine) < 0) goto cleanup; - vboxDumpSharedFolders(def, data, machine); + if (vboxDumpSharedFolders(def, data, machine) < 0) + goto cleanup; vboxDumpNetwork(def, data, machine, networkAdapterCount); vboxDumpAudio(def, data, machine); -- 2.13.6
Instead of using def->nets every time, use a temporary pointer. This will allow splitting out the per-adapter code. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/vbox/vbox_common.c | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c index cc7772f25..052655ca7 100644 --- a/src/vbox/vbox_common.c +++ b/src/vbox/vbox_common.c @@ -3726,6 +3726,7 @@ vboxDumpNetwork(virDomainDefPtr def, vboxDriverPtr data, IMachine *machine, PRUi /* Now get the details about the network cards here */ for (i = 0; netAdpIncCnt < def->nnets && i < networkAdapterCount; i++) { INetworkAdapter *adapter = NULL; + virDomainNetDefPtr net = def->nets[netAdpIncCnt]; gVBoxAPI.UIMachine.GetNetworkAdapter(machine, i, &adapter); if (adapter) { @@ -3742,18 +3743,18 @@ vboxDumpNetwork(virDomainDefPtr def, vboxDriverPtr data, IMachine *machine, PRUi gVBoxAPI.UINetworkAdapter.GetAttachmentType(adapter, &attachmentType); if (attachmentType == NetworkAttachmentType_NAT) { - def->nets[netAdpIncCnt]->type = VIR_DOMAIN_NET_TYPE_USER; + net->type = VIR_DOMAIN_NET_TYPE_USER; } else if (attachmentType == NetworkAttachmentType_Bridged) { PRUnichar *hostIntUtf16 = NULL; char *hostInt = NULL; - def->nets[netAdpIncCnt]->type = VIR_DOMAIN_NET_TYPE_BRIDGE; + net->type = VIR_DOMAIN_NET_TYPE_BRIDGE; gVBoxAPI.UINetworkAdapter.GetBridgedInterface(adapter, &hostIntUtf16); VBOX_UTF16_TO_UTF8(hostIntUtf16, &hostInt); - ignore_value(VIR_STRDUP(def->nets[netAdpIncCnt]->data.bridge.brname, hostInt)); + ignore_value(VIR_STRDUP(net->data.bridge.brname, hostInt)); VBOX_UTF8_FREE(hostInt); VBOX_UTF16_FREE(hostIntUtf16); @@ -3762,12 +3763,12 @@ vboxDumpNetwork(virDomainDefPtr def, vboxDriverPtr data, IMachine *machine, PRUi PRUnichar *intNetUtf16 = NULL; char *intNet = NULL; - def->nets[netAdpIncCnt]->type = VIR_DOMAIN_NET_TYPE_INTERNAL; + net->type = VIR_DOMAIN_NET_TYPE_INTERNAL; gVBoxAPI.UINetworkAdapter.GetInternalNetwork(adapter, &intNetUtf16); VBOX_UTF16_TO_UTF8(intNetUtf16, &intNet); - ignore_value(VIR_STRDUP(def->nets[netAdpIncCnt]->data.internal.name, intNet)); + ignore_value(VIR_STRDUP(net->data.internal.name, intNet)); VBOX_UTF8_FREE(intNet); VBOX_UTF16_FREE(intNetUtf16); @@ -3776,12 +3777,12 @@ vboxDumpNetwork(virDomainDefPtr def, vboxDriverPtr data, IMachine *machine, PRUi PRUnichar *hostIntUtf16 = NULL; char *hostInt = NULL; - def->nets[netAdpIncCnt]->type = VIR_DOMAIN_NET_TYPE_NETWORK; + net->type = VIR_DOMAIN_NET_TYPE_NETWORK; gVBoxAPI.UINetworkAdapter.GetHostOnlyInterface(adapter, &hostIntUtf16); VBOX_UTF16_TO_UTF8(hostIntUtf16, &hostInt); - ignore_value(VIR_STRDUP(def->nets[netAdpIncCnt]->data.network.name, hostInt)); + ignore_value(VIR_STRDUP(net->data.network.name, hostInt)); VBOX_UTF8_FREE(hostInt); VBOX_UTF16_FREE(hostIntUtf16); @@ -3790,24 +3791,24 @@ vboxDumpNetwork(virDomainDefPtr def, vboxDriverPtr data, IMachine *machine, PRUi /* default to user type i.e. NAT in VirtualBox if this * dump is ever used to create a machine. */ - def->nets[netAdpIncCnt]->type = VIR_DOMAIN_NET_TYPE_USER; + net->type = VIR_DOMAIN_NET_TYPE_USER; } gVBoxAPI.UINetworkAdapter.GetAdapterType(adapter, &adapterType); if (adapterType == NetworkAdapterType_Am79C970A) { - ignore_value(VIR_STRDUP(def->nets[netAdpIncCnt]->model, "Am79C970A")); + ignore_value(VIR_STRDUP(net->model, "Am79C970A")); } else if (adapterType == NetworkAdapterType_Am79C973) { - ignore_value(VIR_STRDUP(def->nets[netAdpIncCnt]->model, "Am79C973")); + ignore_value(VIR_STRDUP(net->model, "Am79C973")); } else if (adapterType == NetworkAdapterType_I82540EM) { - ignore_value(VIR_STRDUP(def->nets[netAdpIncCnt]->model, "82540EM")); + ignore_value(VIR_STRDUP(net->model, "82540EM")); } else if (adapterType == NetworkAdapterType_I82545EM) { - ignore_value(VIR_STRDUP(def->nets[netAdpIncCnt]->model, "82545EM")); + ignore_value(VIR_STRDUP(net->model, "82545EM")); } else if (adapterType == NetworkAdapterType_I82543GC) { - ignore_value(VIR_STRDUP(def->nets[netAdpIncCnt]->model, "82543GC")); + ignore_value(VIR_STRDUP(net->model, "82543GC")); } else if (gVBoxAPI.APIVersion >= 3000051 && adapterType == NetworkAdapterType_Virtio) { /* Only vbox 3.1 and later support NetworkAdapterType_Virto */ - ignore_value(VIR_STRDUP(def->nets[netAdpIncCnt]->model, "virtio")); + ignore_value(VIR_STRDUP(net->model, "virtio")); } gVBoxAPI.UINetworkAdapter.GetMACAddress(adapter, &MACAddressUtf16); @@ -3819,8 +3820,7 @@ vboxDumpNetwork(virDomainDefPtr def, vboxDriverPtr data, IMachine *machine, PRUi MACAddress[8], MACAddress[9], MACAddress[10], MACAddress[11]); /* XXX some real error handling here some day ... */ - ignore_value(virMacAddrParse(macaddr, - &def->nets[netAdpIncCnt]->mac)); + ignore_value(virMacAddrParse(macaddr, &net->mac)); netAdpIncCnt++; -- 2.13.6
Free up 'vboxDumpNetwork' for dumping single network. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/vbox/vbox_common.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c index 052655ca7..03266557a 100644 --- a/src/vbox/vbox_common.c +++ b/src/vbox/vbox_common.c @@ -3695,7 +3695,7 @@ vboxDumpSharedFolders(virDomainDefPtr def, vboxDriverPtr data, IMachine *machine } static void -vboxDumpNetwork(virDomainDefPtr def, vboxDriverPtr data, IMachine *machine, PRUint32 networkAdapterCount) +vboxDumpNetworks(virDomainDefPtr def, vboxDriverPtr data, IMachine *machine, PRUint32 networkAdapterCount) { PRUint32 netAdpIncCnt = 0; size_t i = 0; @@ -4188,7 +4188,7 @@ static char *vboxDomainGetXMLDesc(virDomainPtr dom, unsigned int flags) if (vboxDumpSharedFolders(def, data, machine) < 0) goto cleanup; - vboxDumpNetwork(def, data, machine, networkAdapterCount); + vboxDumpNetworks(def, data, machine, networkAdapterCount); vboxDumpAudio(def, data, machine); if (vboxDumpSerial(def, data, machine, serialPortCount) < 0) -- 2.13.6
Split out per-adapter code from vboxDumpNetworks. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/vbox/vbox_common.c | 186 +++++++++++++++++++++++++------------------------ 1 file changed, 96 insertions(+), 90 deletions(-) diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c index 03266557a..295f48376 100644 --- a/src/vbox/vbox_common.c +++ b/src/vbox/vbox_common.c @@ -3695,6 +3695,101 @@ vboxDumpSharedFolders(virDomainDefPtr def, vboxDriverPtr data, IMachine *machine } static void +vboxDumpNetwork(virDomainNetDefPtr net, vboxDriverPtr data, INetworkAdapter *adapter) +{ + PRUint32 attachmentType = NetworkAttachmentType_Null; + PRUint32 adapterType = NetworkAdapterType_Null; + PRUnichar *MACAddressUtf16 = NULL; + char *MACAddress = NULL; + char macaddr[VIR_MAC_STRING_BUFLEN] = {0}; + + gVBoxAPI.UINetworkAdapter.GetAttachmentType(adapter, &attachmentType); + if (attachmentType == NetworkAttachmentType_NAT) { + + net->type = VIR_DOMAIN_NET_TYPE_USER; + + } else if (attachmentType == NetworkAttachmentType_Bridged) { + PRUnichar *hostIntUtf16 = NULL; + char *hostInt = NULL; + + net->type = VIR_DOMAIN_NET_TYPE_BRIDGE; + + gVBoxAPI.UINetworkAdapter.GetBridgedInterface(adapter, &hostIntUtf16); + + VBOX_UTF16_TO_UTF8(hostIntUtf16, &hostInt); + ignore_value(VIR_STRDUP(net->data.bridge.brname, hostInt)); + + VBOX_UTF8_FREE(hostInt); + VBOX_UTF16_FREE(hostIntUtf16); + + } else if (attachmentType == NetworkAttachmentType_Internal) { + PRUnichar *intNetUtf16 = NULL; + char *intNet = NULL; + + net->type = VIR_DOMAIN_NET_TYPE_INTERNAL; + + gVBoxAPI.UINetworkAdapter.GetInternalNetwork(adapter, &intNetUtf16); + + VBOX_UTF16_TO_UTF8(intNetUtf16, &intNet); + ignore_value(VIR_STRDUP(net->data.internal.name, intNet)); + + VBOX_UTF8_FREE(intNet); + VBOX_UTF16_FREE(intNetUtf16); + + } else if (attachmentType == NetworkAttachmentType_HostOnly) { + PRUnichar *hostIntUtf16 = NULL; + char *hostInt = NULL; + + net->type = VIR_DOMAIN_NET_TYPE_NETWORK; + + gVBoxAPI.UINetworkAdapter.GetHostOnlyInterface(adapter, &hostIntUtf16); + + VBOX_UTF16_TO_UTF8(hostIntUtf16, &hostInt); + ignore_value(VIR_STRDUP(net->data.network.name, hostInt)); + + VBOX_UTF8_FREE(hostInt); + VBOX_UTF16_FREE(hostIntUtf16); + + } else { + /* default to user type i.e. NAT in VirtualBox if this + * dump is ever used to create a machine. + */ + net->type = VIR_DOMAIN_NET_TYPE_USER; + } + + gVBoxAPI.UINetworkAdapter.GetAdapterType(adapter, &adapterType); + if (adapterType == NetworkAdapterType_Am79C970A) { + ignore_value(VIR_STRDUP(net->model, "Am79C970A")); + } else if (adapterType == NetworkAdapterType_Am79C973) { + ignore_value(VIR_STRDUP(net->model, "Am79C973")); + } else if (adapterType == NetworkAdapterType_I82540EM) { + ignore_value(VIR_STRDUP(net->model, "82540EM")); + } else if (adapterType == NetworkAdapterType_I82545EM) { + ignore_value(VIR_STRDUP(net->model, "82545EM")); + } else if (adapterType == NetworkAdapterType_I82543GC) { + ignore_value(VIR_STRDUP(net->model, "82543GC")); + } else if (gVBoxAPI.APIVersion >= 3000051 && + adapterType == NetworkAdapterType_Virtio) { + /* Only vbox 3.1 and later support NetworkAdapterType_Virto */ + ignore_value(VIR_STRDUP(net->model, "virtio")); + } + + gVBoxAPI.UINetworkAdapter.GetMACAddress(adapter, &MACAddressUtf16); + VBOX_UTF16_TO_UTF8(MACAddressUtf16, &MACAddress); + snprintf(macaddr, VIR_MAC_STRING_BUFLEN, + "%c%c:%c%c:%c%c:%c%c:%c%c:%c%c", + MACAddress[0], MACAddress[1], MACAddress[2], MACAddress[3], + MACAddress[4], MACAddress[5], MACAddress[6], MACAddress[7], + MACAddress[8], MACAddress[9], MACAddress[10], MACAddress[11]); + + /* XXX some real error handling here some day ... */ + ignore_value(virMacAddrParse(macaddr, &net->mac)); + + VBOX_UTF16_FREE(MACAddressUtf16); + VBOX_UTF8_FREE(MACAddress); +} + +static void vboxDumpNetworks(virDomainDefPtr def, vboxDriverPtr data, IMachine *machine, PRUint32 networkAdapterCount) { PRUint32 netAdpIncCnt = 0; @@ -3734,98 +3829,9 @@ vboxDumpNetworks(virDomainDefPtr def, vboxDriverPtr data, IMachine *machine, PRU gVBoxAPI.UINetworkAdapter.GetEnabled(adapter, &enabled); if (enabled) { - PRUint32 attachmentType = NetworkAttachmentType_Null; - PRUint32 adapterType = NetworkAdapterType_Null; - PRUnichar *MACAddressUtf16 = NULL; - char *MACAddress = NULL; - char macaddr[VIR_MAC_STRING_BUFLEN] = {0}; - - gVBoxAPI.UINetworkAdapter.GetAttachmentType(adapter, &attachmentType); - if (attachmentType == NetworkAttachmentType_NAT) { - - net->type = VIR_DOMAIN_NET_TYPE_USER; - - } else if (attachmentType == NetworkAttachmentType_Bridged) { - PRUnichar *hostIntUtf16 = NULL; - char *hostInt = NULL; - - net->type = VIR_DOMAIN_NET_TYPE_BRIDGE; - - gVBoxAPI.UINetworkAdapter.GetBridgedInterface(adapter, &hostIntUtf16); - - VBOX_UTF16_TO_UTF8(hostIntUtf16, &hostInt); - ignore_value(VIR_STRDUP(net->data.bridge.brname, hostInt)); - - VBOX_UTF8_FREE(hostInt); - VBOX_UTF16_FREE(hostIntUtf16); - - } else if (attachmentType == NetworkAttachmentType_Internal) { - PRUnichar *intNetUtf16 = NULL; - char *intNet = NULL; - - net->type = VIR_DOMAIN_NET_TYPE_INTERNAL; - - gVBoxAPI.UINetworkAdapter.GetInternalNetwork(adapter, &intNetUtf16); - - VBOX_UTF16_TO_UTF8(intNetUtf16, &intNet); - ignore_value(VIR_STRDUP(net->data.internal.name, intNet)); - - VBOX_UTF8_FREE(intNet); - VBOX_UTF16_FREE(intNetUtf16); - - } else if (attachmentType == NetworkAttachmentType_HostOnly) { - PRUnichar *hostIntUtf16 = NULL; - char *hostInt = NULL; - - net->type = VIR_DOMAIN_NET_TYPE_NETWORK; - - gVBoxAPI.UINetworkAdapter.GetHostOnlyInterface(adapter, &hostIntUtf16); - - VBOX_UTF16_TO_UTF8(hostIntUtf16, &hostInt); - ignore_value(VIR_STRDUP(net->data.network.name, hostInt)); - - VBOX_UTF8_FREE(hostInt); - VBOX_UTF16_FREE(hostIntUtf16); - - } else { - /* default to user type i.e. NAT in VirtualBox if this - * dump is ever used to create a machine. - */ - net->type = VIR_DOMAIN_NET_TYPE_USER; - } - - gVBoxAPI.UINetworkAdapter.GetAdapterType(adapter, &adapterType); - if (adapterType == NetworkAdapterType_Am79C970A) { - ignore_value(VIR_STRDUP(net->model, "Am79C970A")); - } else if (adapterType == NetworkAdapterType_Am79C973) { - ignore_value(VIR_STRDUP(net->model, "Am79C973")); - } else if (adapterType == NetworkAdapterType_I82540EM) { - ignore_value(VIR_STRDUP(net->model, "82540EM")); - } else if (adapterType == NetworkAdapterType_I82545EM) { - ignore_value(VIR_STRDUP(net->model, "82545EM")); - } else if (adapterType == NetworkAdapterType_I82543GC) { - ignore_value(VIR_STRDUP(net->model, "82543GC")); - } else if (gVBoxAPI.APIVersion >= 3000051 && - adapterType == NetworkAdapterType_Virtio) { - /* Only vbox 3.1 and later support NetworkAdapterType_Virto */ - ignore_value(VIR_STRDUP(net->model, "virtio")); - } - - gVBoxAPI.UINetworkAdapter.GetMACAddress(adapter, &MACAddressUtf16); - VBOX_UTF16_TO_UTF8(MACAddressUtf16, &MACAddress); - snprintf(macaddr, VIR_MAC_STRING_BUFLEN, - "%c%c:%c%c:%c%c:%c%c:%c%c:%c%c", - MACAddress[0], MACAddress[1], MACAddress[2], MACAddress[3], - MACAddress[4], MACAddress[5], MACAddress[6], MACAddress[7], - MACAddress[8], MACAddress[9], MACAddress[10], MACAddress[11]); - - /* XXX some real error handling here some day ... */ - ignore_value(virMacAddrParse(macaddr, &net->mac)); + vboxDumpNetwork(net, data, adapter); netAdpIncCnt++; - - VBOX_UTF16_FREE(MACAddressUtf16); - VBOX_UTF8_FREE(MACAddress); } VBOX_RELEASE(adapter); -- 2.13.6
The 'enabled' bool is initialized to false, there is no need to nest the conditions. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/vbox/vbox_common.c | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c index 295f48376..1d38001f9 100644 --- a/src/vbox/vbox_common.c +++ b/src/vbox/vbox_common.c @@ -3822,20 +3822,19 @@ vboxDumpNetworks(virDomainDefPtr def, vboxDriverPtr data, IMachine *machine, PRU for (i = 0; netAdpIncCnt < def->nnets && i < networkAdapterCount; i++) { INetworkAdapter *adapter = NULL; virDomainNetDefPtr net = def->nets[netAdpIncCnt]; + PRBool enabled = PR_FALSE; gVBoxAPI.UIMachine.GetNetworkAdapter(machine, i, &adapter); - if (adapter) { - PRBool enabled = PR_FALSE; - + if (adapter) gVBoxAPI.UINetworkAdapter.GetEnabled(adapter, &enabled); - if (enabled) { - vboxDumpNetwork(net, data, adapter); - netAdpIncCnt++; - } + if (enabled) { + vboxDumpNetwork(net, data, adapter); - VBOX_RELEASE(adapter); + netAdpIncCnt++; } + + VBOX_RELEASE(adapter); } } -- 2.13.6
Move the allocation from vboxDumpNetworks inside vboxDumpNetwork. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/vbox/vbox_common.c | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c index 1d38001f9..2eb7af4ba 100644 --- a/src/vbox/vbox_common.c +++ b/src/vbox/vbox_common.c @@ -3694,14 +3694,18 @@ vboxDumpSharedFolders(virDomainDefPtr def, vboxDriverPtr data, IMachine *machine return ret; } -static void -vboxDumpNetwork(virDomainNetDefPtr net, vboxDriverPtr data, INetworkAdapter *adapter) +static virDomainNetDefPtr +vboxDumpNetwork(vboxDriverPtr data, INetworkAdapter *adapter) { PRUint32 attachmentType = NetworkAttachmentType_Null; PRUint32 adapterType = NetworkAdapterType_Null; PRUnichar *MACAddressUtf16 = NULL; char *MACAddress = NULL; char macaddr[VIR_MAC_STRING_BUFLEN] = {0}; + virDomainNetDefPtr net = NULL; + + if (VIR_ALLOC(net) < 0) + return NULL; gVBoxAPI.UINetworkAdapter.GetAttachmentType(adapter, &attachmentType); if (attachmentType == NetworkAttachmentType_NAT) { @@ -3787,6 +3791,7 @@ vboxDumpNetwork(virDomainNetDefPtr net, vboxDriverPtr data, INetworkAdapter *ada VBOX_UTF16_FREE(MACAddressUtf16); VBOX_UTF8_FREE(MACAddress); + return net; } static void @@ -3813,15 +3818,13 @@ vboxDumpNetworks(virDomainDefPtr def, vboxDriverPtr data, IMachine *machine, PRU } /* Allocate memory for the networkcards which are enabled */ - if ((def->nnets > 0) && (VIR_ALLOC_N(def->nets, def->nnets) >= 0)) { - for (i = 0; i < def->nnets; i++) - ignore_value(VIR_ALLOC(def->nets[i])); - } + if (def->nnets > 0) + ignore_value(VIR_ALLOC_N(def->nets, def->nnets)); /* Now get the details about the network cards here */ for (i = 0; netAdpIncCnt < def->nnets && i < networkAdapterCount; i++) { INetworkAdapter *adapter = NULL; - virDomainNetDefPtr net = def->nets[netAdpIncCnt]; + virDomainNetDefPtr net = NULL; PRBool enabled = PR_FALSE; gVBoxAPI.UIMachine.GetNetworkAdapter(machine, i, &adapter); @@ -3829,9 +3832,8 @@ vboxDumpNetworks(virDomainDefPtr def, vboxDriverPtr data, IMachine *machine, PRU gVBoxAPI.UINetworkAdapter.GetEnabled(adapter, &enabled); if (enabled) { - vboxDumpNetwork(net, data, adapter); - - netAdpIncCnt++; + net = vboxDumpNetwork(data, adapter); + def->nets[netAdpIncCnt++] = net; } VBOX_RELEASE(adapter); -- 2.13.6
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/vbox/vbox_common.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c index 2eb7af4ba..c730c0cc0 100644 --- a/src/vbox/vbox_common.c +++ b/src/vbox/vbox_common.c @@ -3799,7 +3799,7 @@ vboxDumpNetworks(virDomainDefPtr def, vboxDriverPtr data, IMachine *machine, PRU { PRUint32 netAdpIncCnt = 0; size_t i = 0; - /* dump network cards if present */ + def->nnets = 0; /* Get which network cards are enabled */ for (i = 0; i < networkAdapterCount; i++) { -- 2.13.6
Use VIR_APPEND_ELEMENT instead and change the return type to int to catch allocation errors. This removes the need to figure out the adapter count upfront. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/vbox/vbox_common.c | 35 +++++++++-------------------------- 1 file changed, 9 insertions(+), 26 deletions(-) diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c index c730c0cc0..c807d2965 100644 --- a/src/vbox/vbox_common.c +++ b/src/vbox/vbox_common.c @@ -3794,36 +3794,13 @@ vboxDumpNetwork(vboxDriverPtr data, INetworkAdapter *adapter) return net; } -static void +static int vboxDumpNetworks(virDomainDefPtr def, vboxDriverPtr data, IMachine *machine, PRUint32 networkAdapterCount) { - PRUint32 netAdpIncCnt = 0; size_t i = 0; - def->nnets = 0; - /* Get which network cards are enabled */ for (i = 0; i < networkAdapterCount; i++) { INetworkAdapter *adapter = NULL; - - gVBoxAPI.UIMachine.GetNetworkAdapter(machine, i, &adapter); - if (adapter) { - PRBool enabled = PR_FALSE; - - gVBoxAPI.UINetworkAdapter.GetEnabled(adapter, &enabled); - if (enabled) - def->nnets++; - - VBOX_RELEASE(adapter); - } - } - - /* Allocate memory for the networkcards which are enabled */ - if (def->nnets > 0) - ignore_value(VIR_ALLOC_N(def->nets, def->nnets)); - - /* Now get the details about the network cards here */ - for (i = 0; netAdpIncCnt < def->nnets && i < networkAdapterCount; i++) { - INetworkAdapter *adapter = NULL; virDomainNetDefPtr net = NULL; PRBool enabled = PR_FALSE; @@ -3833,11 +3810,16 @@ vboxDumpNetworks(virDomainDefPtr def, vboxDriverPtr data, IMachine *machine, PRU if (enabled) { net = vboxDumpNetwork(data, adapter); - def->nets[netAdpIncCnt++] = net; + if (VIR_APPEND_ELEMENT(def->nets, def->nnets, net) < 0) { + VBOX_RELEASE(adapter); + return -1; + } } VBOX_RELEASE(adapter); } + + return 0; } static void @@ -4195,7 +4177,8 @@ static char *vboxDomainGetXMLDesc(virDomainPtr dom, unsigned int flags) if (vboxDumpSharedFolders(def, data, machine) < 0) goto cleanup; - vboxDumpNetworks(def, data, machine, networkAdapterCount); + if (vboxDumpNetworks(def, data, machine, networkAdapterCount) < 0) + goto cleanup; vboxDumpAudio(def, data, machine); if (vboxDumpSerial(def, data, machine, serialPortCount) < 0) -- 2.13.6
Use the virMacAddrParse helper that does not require colon-separated values instead of using extra code to format it that way. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/vbox/vbox_common.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c index c807d2965..3099e20c5 100644 --- a/src/vbox/vbox_common.c +++ b/src/vbox/vbox_common.c @@ -3701,7 +3701,6 @@ vboxDumpNetwork(vboxDriverPtr data, INetworkAdapter *adapter) PRUint32 adapterType = NetworkAdapterType_Null; PRUnichar *MACAddressUtf16 = NULL; char *MACAddress = NULL; - char macaddr[VIR_MAC_STRING_BUFLEN] = {0}; virDomainNetDefPtr net = NULL; if (VIR_ALLOC(net) < 0) @@ -3780,18 +3779,19 @@ vboxDumpNetwork(vboxDriverPtr data, INetworkAdapter *adapter) gVBoxAPI.UINetworkAdapter.GetMACAddress(adapter, &MACAddressUtf16); VBOX_UTF16_TO_UTF8(MACAddressUtf16, &MACAddress); - snprintf(macaddr, VIR_MAC_STRING_BUFLEN, - "%c%c:%c%c:%c%c:%c%c:%c%c:%c%c", - MACAddress[0], MACAddress[1], MACAddress[2], MACAddress[3], - MACAddress[4], MACAddress[5], MACAddress[6], MACAddress[7], - MACAddress[8], MACAddress[9], MACAddress[10], MACAddress[11]); + VBOX_UTF16_FREE(MACAddressUtf16); - /* XXX some real error handling here some day ... */ - ignore_value(virMacAddrParse(macaddr, &net->mac)); + if (virMacAddrParseHex(MACAddress, &net->mac) < 0) { + VBOX_UTF8_FREE(MACAddress); + goto error; + } - VBOX_UTF16_FREE(MACAddressUtf16); VBOX_UTF8_FREE(MACAddress); return net; + + error: + virDomainNetDefFree(net); + return NULL; } static int -- 2.13.6
There is a pattern of using two temporary utf16/utf8 variables for every value we get from VirtualBox and put in the domain definition right away. Reuse the same variable name to improve the chances of getting the function on one screen. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/vbox/vbox_common.c | 29 +++++++++++++---------------- 1 file changed, 13 insertions(+), 16 deletions(-) diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c index 3099e20c5..dc12bc662 100644 --- a/src/vbox/vbox_common.c +++ b/src/vbox/vbox_common.c @@ -3699,7 +3699,7 @@ vboxDumpNetwork(vboxDriverPtr data, INetworkAdapter *adapter) { PRUint32 attachmentType = NetworkAttachmentType_Null; PRUint32 adapterType = NetworkAdapterType_Null; - PRUnichar *MACAddressUtf16 = NULL; + PRUnichar *utf16 = NULL; char *MACAddress = NULL; virDomainNetDefPtr net = NULL; @@ -3712,46 +3712,43 @@ vboxDumpNetwork(vboxDriverPtr data, INetworkAdapter *adapter) net->type = VIR_DOMAIN_NET_TYPE_USER; } else if (attachmentType == NetworkAttachmentType_Bridged) { - PRUnichar *hostIntUtf16 = NULL; char *hostInt = NULL; net->type = VIR_DOMAIN_NET_TYPE_BRIDGE; - gVBoxAPI.UINetworkAdapter.GetBridgedInterface(adapter, &hostIntUtf16); + gVBoxAPI.UINetworkAdapter.GetBridgedInterface(adapter, &utf16); - VBOX_UTF16_TO_UTF8(hostIntUtf16, &hostInt); + VBOX_UTF16_TO_UTF8(utf16, &hostInt); ignore_value(VIR_STRDUP(net->data.bridge.brname, hostInt)); VBOX_UTF8_FREE(hostInt); - VBOX_UTF16_FREE(hostIntUtf16); + VBOX_UTF16_FREE(utf16); } else if (attachmentType == NetworkAttachmentType_Internal) { - PRUnichar *intNetUtf16 = NULL; char *intNet = NULL; net->type = VIR_DOMAIN_NET_TYPE_INTERNAL; - gVBoxAPI.UINetworkAdapter.GetInternalNetwork(adapter, &intNetUtf16); + gVBoxAPI.UINetworkAdapter.GetInternalNetwork(adapter, &utf16); - VBOX_UTF16_TO_UTF8(intNetUtf16, &intNet); + VBOX_UTF16_TO_UTF8(utf16, &intNet); ignore_value(VIR_STRDUP(net->data.internal.name, intNet)); VBOX_UTF8_FREE(intNet); - VBOX_UTF16_FREE(intNetUtf16); + VBOX_UTF16_FREE(utf16); } else if (attachmentType == NetworkAttachmentType_HostOnly) { - PRUnichar *hostIntUtf16 = NULL; char *hostInt = NULL; net->type = VIR_DOMAIN_NET_TYPE_NETWORK; - gVBoxAPI.UINetworkAdapter.GetHostOnlyInterface(adapter, &hostIntUtf16); + gVBoxAPI.UINetworkAdapter.GetHostOnlyInterface(adapter, &utf16); - VBOX_UTF16_TO_UTF8(hostIntUtf16, &hostInt); + VBOX_UTF16_TO_UTF8(utf16, &hostInt); ignore_value(VIR_STRDUP(net->data.network.name, hostInt)); VBOX_UTF8_FREE(hostInt); - VBOX_UTF16_FREE(hostIntUtf16); + VBOX_UTF16_FREE(utf16); } else { /* default to user type i.e. NAT in VirtualBox if this @@ -3777,9 +3774,9 @@ vboxDumpNetwork(vboxDriverPtr data, INetworkAdapter *adapter) ignore_value(VIR_STRDUP(net->model, "virtio")); } - gVBoxAPI.UINetworkAdapter.GetMACAddress(adapter, &MACAddressUtf16); - VBOX_UTF16_TO_UTF8(MACAddressUtf16, &MACAddress); - VBOX_UTF16_FREE(MACAddressUtf16); + gVBoxAPI.UINetworkAdapter.GetMACAddress(adapter, &utf16); + VBOX_UTF16_TO_UTF8(utf16, &MACAddress); + VBOX_UTF16_FREE(utf16); if (virMacAddrParseHex(MACAddress, &net->mac) < 0) { VBOX_UTF8_FREE(MACAddress); -- 2.13.6
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/vbox/vbox_common.c | 34 ++++++++++++++-------------------- 1 file changed, 14 insertions(+), 20 deletions(-) diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c index dc12bc662..2943c534d 100644 --- a/src/vbox/vbox_common.c +++ b/src/vbox/vbox_common.c @@ -3700,7 +3700,7 @@ vboxDumpNetwork(vboxDriverPtr data, INetworkAdapter *adapter) PRUint32 attachmentType = NetworkAttachmentType_Null; PRUint32 adapterType = NetworkAdapterType_Null; PRUnichar *utf16 = NULL; - char *MACAddress = NULL; + char *utf8 = NULL; virDomainNetDefPtr net = NULL; if (VIR_ALLOC(net) < 0) @@ -3712,42 +3712,36 @@ vboxDumpNetwork(vboxDriverPtr data, INetworkAdapter *adapter) net->type = VIR_DOMAIN_NET_TYPE_USER; } else if (attachmentType == NetworkAttachmentType_Bridged) { - char *hostInt = NULL; - net->type = VIR_DOMAIN_NET_TYPE_BRIDGE; gVBoxAPI.UINetworkAdapter.GetBridgedInterface(adapter, &utf16); - VBOX_UTF16_TO_UTF8(utf16, &hostInt); - ignore_value(VIR_STRDUP(net->data.bridge.brname, hostInt)); + VBOX_UTF16_TO_UTF8(utf16, &utf8); + ignore_value(VIR_STRDUP(net->data.bridge.brname, utf8)); - VBOX_UTF8_FREE(hostInt); + VBOX_UTF8_FREE(utf8); VBOX_UTF16_FREE(utf16); } else if (attachmentType == NetworkAttachmentType_Internal) { - char *intNet = NULL; - net->type = VIR_DOMAIN_NET_TYPE_INTERNAL; gVBoxAPI.UINetworkAdapter.GetInternalNetwork(adapter, &utf16); - VBOX_UTF16_TO_UTF8(utf16, &intNet); - ignore_value(VIR_STRDUP(net->data.internal.name, intNet)); + VBOX_UTF16_TO_UTF8(utf16, &utf8); + ignore_value(VIR_STRDUP(net->data.internal.name, utf8)); - VBOX_UTF8_FREE(intNet); + VBOX_UTF8_FREE(utf8); VBOX_UTF16_FREE(utf16); } else if (attachmentType == NetworkAttachmentType_HostOnly) { - char *hostInt = NULL; - net->type = VIR_DOMAIN_NET_TYPE_NETWORK; gVBoxAPI.UINetworkAdapter.GetHostOnlyInterface(adapter, &utf16); - VBOX_UTF16_TO_UTF8(utf16, &hostInt); - ignore_value(VIR_STRDUP(net->data.network.name, hostInt)); + VBOX_UTF16_TO_UTF8(utf16, &utf8); + ignore_value(VIR_STRDUP(net->data.network.name, utf8)); - VBOX_UTF8_FREE(hostInt); + VBOX_UTF8_FREE(utf8); VBOX_UTF16_FREE(utf16); } else { @@ -3775,15 +3769,15 @@ vboxDumpNetwork(vboxDriverPtr data, INetworkAdapter *adapter) } gVBoxAPI.UINetworkAdapter.GetMACAddress(adapter, &utf16); - VBOX_UTF16_TO_UTF8(utf16, &MACAddress); + VBOX_UTF16_TO_UTF8(utf16, &utf8); VBOX_UTF16_FREE(utf16); - if (virMacAddrParseHex(MACAddress, &net->mac) < 0) { - VBOX_UTF8_FREE(MACAddress); + if (virMacAddrParseHex(utf8, &net->mac) < 0) { + VBOX_UTF8_FREE(utf8); goto error; } - VBOX_UTF8_FREE(MACAddress); + VBOX_UTF8_FREE(utf8); return net; error: -- 2.13.6
Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/vbox/vbox_common.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c index 2943c534d..8f5f04efb 100644 --- a/src/vbox/vbox_common.c +++ b/src/vbox/vbox_common.c @@ -3707,11 +3707,13 @@ vboxDumpNetwork(vboxDriverPtr data, INetworkAdapter *adapter) return NULL; gVBoxAPI.UINetworkAdapter.GetAttachmentType(adapter, &attachmentType); - if (attachmentType == NetworkAttachmentType_NAT) { + switch (attachmentType) { + case NetworkAttachmentType_NAT: net->type = VIR_DOMAIN_NET_TYPE_USER; + break; - } else if (attachmentType == NetworkAttachmentType_Bridged) { + case NetworkAttachmentType_Bridged: net->type = VIR_DOMAIN_NET_TYPE_BRIDGE; gVBoxAPI.UINetworkAdapter.GetBridgedInterface(adapter, &utf16); @@ -3721,8 +3723,9 @@ vboxDumpNetwork(vboxDriverPtr data, INetworkAdapter *adapter) VBOX_UTF8_FREE(utf8); VBOX_UTF16_FREE(utf16); + break; - } else if (attachmentType == NetworkAttachmentType_Internal) { + case NetworkAttachmentType_Internal: net->type = VIR_DOMAIN_NET_TYPE_INTERNAL; gVBoxAPI.UINetworkAdapter.GetInternalNetwork(adapter, &utf16); @@ -3732,8 +3735,9 @@ vboxDumpNetwork(vboxDriverPtr data, INetworkAdapter *adapter) VBOX_UTF8_FREE(utf8); VBOX_UTF16_FREE(utf16); + break; - } else if (attachmentType == NetworkAttachmentType_HostOnly) { + case NetworkAttachmentType_HostOnly: net->type = VIR_DOMAIN_NET_TYPE_NETWORK; gVBoxAPI.UINetworkAdapter.GetHostOnlyInterface(adapter, &utf16); @@ -3743,8 +3747,9 @@ vboxDumpNetwork(vboxDriverPtr data, INetworkAdapter *adapter) VBOX_UTF8_FREE(utf8); VBOX_UTF16_FREE(utf16); + break; - } else { + default: /* default to user type i.e. NAT in VirtualBox if this * dump is ever used to create a machine. */ -- 2.13.6
We can steal the strings instead of creating more copies. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/vbox/vbox_common.c | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c index 8f5f04efb..1a413e4ac 100644 --- a/src/vbox/vbox_common.c +++ b/src/vbox/vbox_common.c @@ -3719,9 +3719,7 @@ vboxDumpNetwork(vboxDriverPtr data, INetworkAdapter *adapter) gVBoxAPI.UINetworkAdapter.GetBridgedInterface(adapter, &utf16); VBOX_UTF16_TO_UTF8(utf16, &utf8); - ignore_value(VIR_STRDUP(net->data.bridge.brname, utf8)); - - VBOX_UTF8_FREE(utf8); + VIR_STEAL_PTR(net->data.bridge.brname, utf8); VBOX_UTF16_FREE(utf16); break; @@ -3731,9 +3729,7 @@ vboxDumpNetwork(vboxDriverPtr data, INetworkAdapter *adapter) gVBoxAPI.UINetworkAdapter.GetInternalNetwork(adapter, &utf16); VBOX_UTF16_TO_UTF8(utf16, &utf8); - ignore_value(VIR_STRDUP(net->data.internal.name, utf8)); - - VBOX_UTF8_FREE(utf8); + VIR_STEAL_PTR(net->data.internal.name, utf8); VBOX_UTF16_FREE(utf16); break; @@ -3743,9 +3739,7 @@ vboxDumpNetwork(vboxDriverPtr data, INetworkAdapter *adapter) gVBoxAPI.UINetworkAdapter.GetHostOnlyInterface(adapter, &utf16); VBOX_UTF16_TO_UTF8(utf16, &utf8); - ignore_value(VIR_STRDUP(net->data.network.name, utf8)); - - VBOX_UTF8_FREE(utf8); + VIR_STEAL_PTR(net->data.network.name, utf8); VBOX_UTF16_FREE(utf16); break; -- 2.13.6
Also return an error when VIR_STRDUP fails. Signed-off-by: Ján Tomko <jtomko@redhat.com> --- src/vbox/vbox_common.c | 36 +++++++++++++++++++++++------------- 1 file changed, 23 insertions(+), 13 deletions(-) diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c index 1a413e4ac..3bcca43d3 100644 --- a/src/vbox/vbox_common.c +++ b/src/vbox/vbox_common.c @@ -3699,6 +3699,7 @@ vboxDumpNetwork(vboxDriverPtr data, INetworkAdapter *adapter) { PRUint32 attachmentType = NetworkAttachmentType_Null; PRUint32 adapterType = NetworkAdapterType_Null; + const char *model = NULL; PRUnichar *utf16 = NULL; char *utf8 = NULL; virDomainNetDefPtr net = NULL; @@ -3751,21 +3752,30 @@ vboxDumpNetwork(vboxDriverPtr data, INetworkAdapter *adapter) } gVBoxAPI.UINetworkAdapter.GetAdapterType(adapter, &adapterType); - if (adapterType == NetworkAdapterType_Am79C970A) { - ignore_value(VIR_STRDUP(net->model, "Am79C970A")); - } else if (adapterType == NetworkAdapterType_Am79C973) { - ignore_value(VIR_STRDUP(net->model, "Am79C973")); - } else if (adapterType == NetworkAdapterType_I82540EM) { - ignore_value(VIR_STRDUP(net->model, "82540EM")); - } else if (adapterType == NetworkAdapterType_I82545EM) { - ignore_value(VIR_STRDUP(net->model, "82545EM")); - } else if (adapterType == NetworkAdapterType_I82543GC) { - ignore_value(VIR_STRDUP(net->model, "82543GC")); - } else if (gVBoxAPI.APIVersion >= 3000051 && - adapterType == NetworkAdapterType_Virtio) { + switch (adapterType) { + case NetworkAdapterType_Am79C970A: + model = "Am79C970A"; + break; + case NetworkAdapterType_Am79C973: + model = "Am79C973"; + break; + case NetworkAdapterType_I82540EM: + model = "82540EM"; + break; + case NetworkAdapterType_I82545EM: + model = "82545EM"; + break; + case NetworkAdapterType_I82543GC: + model = "82543GC"; + break; + case NetworkAdapterType_Virtio: /* Only vbox 3.1 and later support NetworkAdapterType_Virto */ - ignore_value(VIR_STRDUP(net->model, "virtio")); + if (gVBoxAPI.APIVersion >= 3000051) + model = "virtio"; + break; } + if (VIR_STRDUP(net->model, model) < 0) + goto error; gVBoxAPI.UINetworkAdapter.GetMACAddress(adapter, &utf16); VBOX_UTF16_TO_UTF8(utf16, &utf8); -- 2.13.6
On 02/23/2018 09:27 AM, Ján Tomko wrote:
Inspired-by: Laine Stump <laine@laine.org>
How can I *not* at least look at the patches when you call me out like this!
Day-of-the-week: Friday <6>
Ján Tomko (16): vboxDumpSharedFolders: rename non-standard label vboxDumpSharedFolders: remove pointless comment vboxDumpSharedFolders: return a value vboxDumpNetwork: add temp variable for current network vboxDumpNetwork: rename to vboxDumpNetworks vboxDumpNetwork: re-introduce this function vboxDumpNetworks: reduce indentation level vboxDumpNetwork: allocate the network too vboxDumpNetworks: delete pointless comment vboxDumpNetworks: do not allocate def->nets upfront vboxDumpNetwork: use virMacAddrParseHex vboxDumpNetwork: Use a single utf16 variable vboxDumpNetwork: Use a single utf8 temp variable vboxDumpNetwork: use a switch for attachmentType vboxDumpNetwork: use VIR_STEAL_PTR instead of VIR_STRDUP vboxDumpNetwork: use switch for adapterType
src/vbox/vbox_common.c | 243 ++++++++++++++++++++++++------------------------- 1 file changed, 120 insertions(+), 123 deletions(-)
Nice. Where I had whined, you actually took action! :-) I'm unable to test, but I looked through and each patch looks straightforward and sane (there were bits I didn't like (e.g. perpetuating ignore_value() uses), but they were removed in subsequent patches, so all is good. You say that you've actually tested the code, so as long as you've also run make syntax-check and make check: ACK series https://tinyurl.com/y8hxgcg
participants (2)
-
Ján Tomko -
Laine Stump