11:02 a.m.
Check for return values of libvirt's internal APIs in vboxDumpVideo
and vboxDumpDisplay.
Patch 10/13 should fix a complaint by Coverity (untested).
Lots of whitespace changes, -b is your friend.
Ján Tomko (13):
Check return value of vboxDumpVideo
vboxDumpDisplay: reduce indentation level
vboxDumpDisplay: more indentation reducing
vboxDumpDisplay: add addDesktop bool
vboxDumpDisplay: remove extra virReportOOMError
vboxDumpDisplay: split out def->graphics allocation
vboxDumpDisplay: allocate the graphics structure upfront
vboxDumpDisplay: fill out the graphics structure earlier
vboxDumpDisplay: clean up VIR_STRDUP usage
vboxDumpDisplay: check return of virDomainGraphicsListenSetAddress
vboxDumpDisplay: use VIR_APPEND_ELEMENT
vboxDumpDisplay: reuse the keyUtf16 variable
vboxDumpDisplay: remove suspicious strlen
src/vbox/vbox_common.c | 249 ++++++++++++++++++++++---------------------------
1 file changed, 113 insertions(+), 136 deletions(-)
--
2.4.10
11:02 a.m.
New subject: [libvirt] [PATCH 01/13] Check return value of vboxDumpVideo
Error out on allocation failures instead of creating an incomplete
definition.
Fixes a possible crash when def->nvideos is 1, but def->videos is NULL.
---
src/vbox/vbox_common.c | 59 +++++++++++++++++++++++++++-----------------------
1 file changed, 32 insertions(+), 27 deletions(-)
diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c
index d1eb09a..72ba987 100644
--- a/src/vbox/vbox_common.c
+++ b/src/vbox/vbox_common.c
@@ -3258,38 +3258,42 @@ vboxDumpIDEHDDsNew(virDomainDefPtr def, vboxGlobalData *data,
IMachine *machine)
}
}
-static void
+static int
vboxDumpVideo(virDomainDefPtr def, vboxGlobalData *data ATTRIBUTE_UNUSED,
IMachine *machine)
{
/* dump video options vram/2d/3d/directx/etc. */
+ /* the default is: vram is 8MB, One monitor, 3dAccel Off */
+ PRUint32 VRAMSize = 8;
+ PRUint32 monitorCount = 1;
+ PRBool accelerate3DEnabled = PR_FALSE;
+ PRBool accelerate2DEnabled = PR_FALSE;
+
/* Currently supports only one graphics card */
+ if (VIR_ALLOC_N(def->videos, 1) < 0)
+ return -1;
def->nvideos = 1;
- if (VIR_ALLOC_N(def->videos, def->nvideos) >= 0) {
- if (VIR_ALLOC(def->videos[0]) >= 0) {
- /* the default is: vram is 8MB, One monitor, 3dAccel Off */
- PRUint32 VRAMSize = 8;
- PRUint32 monitorCount = 1;
- PRBool accelerate3DEnabled = PR_FALSE;
- PRBool accelerate2DEnabled = PR_FALSE;
-
- gVBoxAPI.UIMachine.GetVRAMSize(machine, &VRAMSize);
- gVBoxAPI.UIMachine.GetMonitorCount(machine, &monitorCount);
- gVBoxAPI.UIMachine.GetAccelerate3DEnabled(machine,
&accelerate3DEnabled);
- if (gVBoxAPI.accelerate2DVideo)
- gVBoxAPI.UIMachine.GetAccelerate2DVideoEnabled(machine,
&accelerate2DEnabled);
-
- def->videos[0]->type = VIR_DOMAIN_VIDEO_TYPE_VBOX;
- def->videos[0]->vram = VRAMSize * 1024;
- def->videos[0]->heads = monitorCount;
- if (VIR_ALLOC(def->videos[0]->accel) >= 0) {
- def->videos[0]->accel->accel3d = accelerate3DEnabled ?
- VIR_TRISTATE_BOOL_YES : VIR_TRISTATE_BOOL_NO;
- def->videos[0]->accel->accel2d = accelerate2DEnabled ?
- VIR_TRISTATE_BOOL_YES : VIR_TRISTATE_BOOL_NO;
- }
- }
- }
+
+ if (VIR_ALLOC(def->videos[0]) < 0)
+ return -1;
+
+ gVBoxAPI.UIMachine.GetVRAMSize(machine, &VRAMSize);
+ gVBoxAPI.UIMachine.GetMonitorCount(machine, &monitorCount);
+ gVBoxAPI.UIMachine.GetAccelerate3DEnabled(machine, &accelerate3DEnabled);
+ if (gVBoxAPI.accelerate2DVideo)
+ gVBoxAPI.UIMachine.GetAccelerate2DVideoEnabled(machine,
&accelerate2DEnabled);
+
+ def->videos[0]->type = VIR_DOMAIN_VIDEO_TYPE_VBOX;
+ def->videos[0]->vram = VRAMSize * 1024;
+ def->videos[0]->heads = monitorCount;
+ if (VIR_ALLOC(def->videos[0]->accel) < 0)
+ return -1;
+ def->videos[0]->accel->accel3d = accelerate3DEnabled ?
+ VIR_TRISTATE_BOOL_YES : VIR_TRISTATE_BOOL_NO;
+ def->videos[0]->accel->accel2d = accelerate2DEnabled ?
+ VIR_TRISTATE_BOOL_YES : VIR_TRISTATE_BOOL_NO;
+
+ return 0;
}
static void
@@ -3967,7 +3971,8 @@ static char *vboxDomainGetXMLDesc(virDomainPtr dom, unsigned int
flags)
* so locatime is always true here */
def->clock.offset = VIR_DOMAIN_CLOCK_OFFSET_LOCALTIME;
- vboxDumpVideo(def, data, machine);
+ if (vboxDumpVideo(def, data, machine) < 0)
+ goto cleanup;
vboxDumpDisplay(def, data, machine);
/* As the medium interface changed from 3.0 to 3.1.
--
2.4.10
10:06 a.m.
New subject: [libvirt] [PATCH 01/13] Check return value of vboxDumpVideo
On 05.02.2016 18:02, Ján Tomko wrote:
Error out on allocation failures instead of creating an incomplete
definition.
Fixes a possible crash when def->nvideos is 1, but def->videos is NULL.
---
src/vbox/vbox_common.c | 59 +++++++++++++++++++++++++++-----------------------
1 file changed, 32 insertions(+), 27 deletions(-)
diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c
index d1eb09a..72ba987 100644
--- a/src/vbox/vbox_common.c
+++ b/src/vbox/vbox_common.c
@@ -3258,38 +3258,42 @@ vboxDumpIDEHDDsNew(virDomainDefPtr def, vboxGlobalData *data,
IMachine *machine)
}
}
-static void
+static int
vboxDumpVideo(virDomainDefPtr def, vboxGlobalData *data ATTRIBUTE_UNUSED,
IMachine *machine)
{
/* dump video options vram/2d/3d/directx/etc. */
+ /* the default is: vram is 8MB, One monitor, 3dAccel Off */
+ PRUint32 VRAMSize = 8;
+ PRUint32 monitorCount = 1;
+ PRBool accelerate3DEnabled = PR_FALSE;
+ PRBool accelerate2DEnabled = PR_FALSE;
+
/* Currently supports only one graphics card */
+ if (VIR_ALLOC_N(def->videos, 1) < 0)
+ return -1;
def->nvideos = 1;
- if (VIR_ALLOC_N(def->videos, def->nvideos) >= 0) {
- if (VIR_ALLOC(def->videos[0]) >= 0) {
- /* the default is: vram is 8MB, One monitor, 3dAccel Off */
- PRUint32 VRAMSize = 8;
- PRUint32 monitorCount = 1;
- PRBool accelerate3DEnabled = PR_FALSE;
- PRBool accelerate2DEnabled = PR_FALSE;
-
- gVBoxAPI.UIMachine.GetVRAMSize(machine, &VRAMSize);
- gVBoxAPI.UIMachine.GetMonitorCount(machine, &monitorCount);
- gVBoxAPI.UIMachine.GetAccelerate3DEnabled(machine,
&accelerate3DEnabled);
- if (gVBoxAPI.accelerate2DVideo)
- gVBoxAPI.UIMachine.GetAccelerate2DVideoEnabled(machine,
&accelerate2DEnabled);
-
- def->videos[0]->type = VIR_DOMAIN_VIDEO_TYPE_VBOX;
- def->videos[0]->vram = VRAMSize * 1024;
- def->videos[0]->heads = monitorCount;
- if (VIR_ALLOC(def->videos[0]->accel) >= 0) {
- def->videos[0]->accel->accel3d = accelerate3DEnabled ?
- VIR_TRISTATE_BOOL_YES : VIR_TRISTATE_BOOL_NO;
- def->videos[0]->accel->accel2d = accelerate2DEnabled ?
- VIR_TRISTATE_BOOL_YES : VIR_TRISTATE_BOOL_NO;
- }
- }
- }
+
+ if (VIR_ALLOC(def->videos[0]) < 0)
+ return -1;
+
+ gVBoxAPI.UIMachine.GetVRAMSize(machine, &VRAMSize);
+ gVBoxAPI.UIMachine.GetMonitorCount(machine, &monitorCount);
+ gVBoxAPI.UIMachine.GetAccelerate3DEnabled(machine, &accelerate3DEnabled);
+ if (gVBoxAPI.accelerate2DVideo)
+ gVBoxAPI.UIMachine.GetAccelerate2DVideoEnabled(machine,
&accelerate2DEnabled);
+
+ def->videos[0]->type = VIR_DOMAIN_VIDEO_TYPE_VBOX;
+ def->videos[0]->vram = VRAMSize * 1024;
+ def->videos[0]->heads = monitorCount;
While touching this, you can drop this weird indentation.
I know we use it throughout whole driver, but one day I'd like to see
that changed too. Same applies for the rest of patches.
+ if (VIR_ALLOC(def->videos[0]->accel) < 0)
+ return -1;
+ def->videos[0]->accel->accel3d = accelerate3DEnabled ?
+ VIR_TRISTATE_BOOL_YES : VIR_TRISTATE_BOOL_NO;
+ def->videos[0]->accel->accel2d = accelerate2DEnabled ?
+ VIR_TRISTATE_BOOL_YES : VIR_TRISTATE_BOOL_NO;
+
+ return 0;
}
static void
@@ -3967,7 +3971,8 @@ static char *vboxDomainGetXMLDesc(virDomainPtr dom, unsigned int
flags)
* so locatime is always true here */
def->clock.offset = VIR_DOMAIN_CLOCK_OFFSET_LOCALTIME;
- vboxDumpVideo(def, data, machine);
+ if (vboxDumpVideo(def, data, machine) < 0)
+ goto cleanup;
vboxDumpDisplay(def, data, machine);
/* As the medium interface changed from 3.0 to 3.1.
ACK
Michal
11:02 a.m.
New subject: [libvirt] [PATCH 02/13] vboxDumpDisplay: reduce indentation level
Use STREQ_NULLABLE instead of deep nesting.
---
src/vbox/vbox_common.c | 75 +++++++++++++++++++++++++-------------------------
1 file changed, 38 insertions(+), 37 deletions(-)
diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c
index 72ba987..ac47728 100644
--- a/src/vbox/vbox_common.c
+++ b/src/vbox/vbox_common.c
@@ -3321,54 +3321,53 @@ vboxDumpDisplay(virDomainDefPtr def, vboxGlobalData *data,
IMachine *machine)
if (valueTypeUtf16) {
VBOX_UTF16_TO_UTF8(valueTypeUtf16, &valueTypeUtf8);
VBOX_UTF16_FREE(valueTypeUtf16);
+ }
- if (STREQ(valueTypeUtf8, "sdl") || STREQ(valueTypeUtf8,
"gui")) {
- PRUnichar *keyDislpayUtf16 = NULL;
- PRUnichar *valueDisplayUtf16 = NULL;
- char *valueDisplayUtf8 = NULL;
+ if (STREQ_NULLABLE(valueTypeUtf8, "sdl") ||
+ STREQ_NULLABLE(valueTypeUtf8, "gui")) {
+ PRUnichar *keyDislpayUtf16 = NULL;
+ PRUnichar *valueDisplayUtf16 = NULL;
+ char *valueDisplayUtf8 = NULL;
- VBOX_UTF8_TO_UTF16("FRONTEND/Display", &keyDislpayUtf16);
- gVBoxAPI.UIMachine.GetExtraData(machine, keyDislpayUtf16,
&valueDisplayUtf16);
- VBOX_UTF16_FREE(keyDislpayUtf16);
+ VBOX_UTF8_TO_UTF16("FRONTEND/Display", &keyDislpayUtf16);
+ gVBoxAPI.UIMachine.GetExtraData(machine, keyDislpayUtf16,
&valueDisplayUtf16);
+ VBOX_UTF16_FREE(keyDislpayUtf16);
- if (valueDisplayUtf16) {
- VBOX_UTF16_TO_UTF8(valueDisplayUtf16, &valueDisplayUtf8);
- VBOX_UTF16_FREE(valueDisplayUtf16);
+ if (valueDisplayUtf16) {
+ VBOX_UTF16_TO_UTF8(valueDisplayUtf16, &valueDisplayUtf8);
+ VBOX_UTF16_FREE(valueDisplayUtf16);
- if (strlen(valueDisplayUtf8) <= 0)
- VBOX_UTF8_FREE(valueDisplayUtf8);
- }
+ if (strlen(valueDisplayUtf8) <= 0)
+ VBOX_UTF8_FREE(valueDisplayUtf8);
+ }
- if (STREQ(valueTypeUtf8, "sdl")) {
- sdlPresent = 1;
- if (VIR_STRDUP(sdlDisplay, valueDisplayUtf8) < 0) {
- /* just don't go to cleanup yet as it is ok to have
- * sdlDisplay as NULL and we check it below if it
- * exist and then only use it there
- */
- }
- totalPresent++;
+ if (STREQ(valueTypeUtf8, "sdl")) {
+ sdlPresent = 1;
+ if (VIR_STRDUP(sdlDisplay, valueDisplayUtf8) < 0) {
+ /* just don't go to cleanup yet as it is ok to have
+ * sdlDisplay as NULL and we check it below if it
+ * exist and then only use it there
+ */
}
+ totalPresent++;
+ }
- if (STREQ(valueTypeUtf8, "gui")) {
- guiPresent = 1;
- if (VIR_STRDUP(guiDisplay, valueDisplayUtf8) < 0) {
- /* just don't go to cleanup yet as it is ok to have
- * guiDisplay as NULL and we check it below if it
- * exist and then only use it there
- */
- }
- totalPresent++;
+ if (STREQ(valueTypeUtf8, "gui")) {
+ guiPresent = 1;
+ if (VIR_STRDUP(guiDisplay, valueDisplayUtf8) < 0) {
+ /* just don't go to cleanup yet as it is ok to have
+ * guiDisplay as NULL and we check it below if it
+ * exist and then only use it there
+ */
}
- VBOX_UTF8_FREE(valueDisplayUtf8);
+ totalPresent++;
}
-
- if (STREQ(valueTypeUtf8, "vrdp"))
- vrdpPresent = 1;
-
- VBOX_UTF8_FREE(valueTypeUtf8);
+ VBOX_UTF8_FREE(valueDisplayUtf8);
}
+ if (STREQ_NULLABLE(valueTypeUtf8, "vrdp"))
+ vrdpPresent = 1;
+
if ((totalPresent > 0) && (VIR_ALLOC_N(def->graphics, totalPresent)
>= 0)) {
if ((guiPresent) && (VIR_ALLOC(def->graphics[def->ngraphics]) >=
0)) {
def->graphics[def->ngraphics]->type =
VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP;
@@ -3441,6 +3440,8 @@ vboxDumpDisplay(virDomainDefPtr def, vboxGlobalData *data, IMachine
*machine)
}
VBOX_RELEASE(VRDxServer);
}
+
+ VBOX_UTF8_FREE(valueTypeUtf8);
}
static void
--
2.4.10
11:02 a.m.
New subject: [libvirt] [PATCH 03/13] vboxDumpDisplay: more indentation reducing
VRDxEnabled is initialized to false. Put the if (VRDxEnabled)
on the top level to reduce nesting.
---
src/vbox/vbox_common.c | 62 +++++++++++++++++++++++++-------------------------
1 file changed, 31 insertions(+), 31 deletions(-)
diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c
index ac47728..1c9d871 100644
--- a/src/vbox/vbox_common.c
+++ b/src/vbox/vbox_common.c
@@ -3398,49 +3398,49 @@ vboxDumpDisplay(virDomainDefPtr def, vboxGlobalData *data,
IMachine *machine)
}
gVBoxAPI.UIMachine.GetVRDxServer(machine, &VRDxServer);
- if (VRDxServer) {
+ if (VRDxServer)
gVBoxAPI.UIVRDxServer.GetEnabled(VRDxServer, &VRDxEnabled);
- if (VRDxEnabled) {
- totalPresent++;
+ if (VRDxEnabled) {
- if ((VIR_REALLOC_N(def->graphics, totalPresent) >= 0) &&
- (VIR_ALLOC(def->graphics[def->ngraphics]) >= 0)) {
- PRUnichar *netAddressUtf16 = NULL;
- char *netAddressUtf8 = NULL;
- PRBool allowMultiConnection = PR_FALSE;
- PRBool reuseSingleConnection = PR_FALSE;
+ totalPresent++;
- gVBoxAPI.UIVRDxServer.GetPorts(data, VRDxServer,
def->graphics[def->ngraphics]);
+ if ((VIR_REALLOC_N(def->graphics, totalPresent) >= 0) &&
+ (VIR_ALLOC(def->graphics[def->ngraphics]) >= 0)) {
+ PRUnichar *netAddressUtf16 = NULL;
+ char *netAddressUtf8 = NULL;
+ PRBool allowMultiConnection = PR_FALSE;
+ PRBool reuseSingleConnection = PR_FALSE;
- def->graphics[def->ngraphics]->type =
VIR_DOMAIN_GRAPHICS_TYPE_RDP;
+ gVBoxAPI.UIVRDxServer.GetPorts(data, VRDxServer,
def->graphics[def->ngraphics]);
- gVBoxAPI.UIVRDxServer.GetNetAddress(data, VRDxServer,
&netAddressUtf16);
- if (netAddressUtf16) {
- VBOX_UTF16_TO_UTF8(netAddressUtf16, &netAddressUtf8);
- if (STRNEQ(netAddressUtf8, ""))
-
virDomainGraphicsListenSetAddress(def->graphics[def->ngraphics], 0,
- netAddressUtf8, -1, true);
- VBOX_UTF16_FREE(netAddressUtf16);
- VBOX_UTF8_FREE(netAddressUtf8);
- }
+ def->graphics[def->ngraphics]->type = VIR_DOMAIN_GRAPHICS_TYPE_RDP;
- gVBoxAPI.UIVRDxServer.GetAllowMultiConnection(VRDxServer,
&allowMultiConnection);
- if (allowMultiConnection)
- def->graphics[def->ngraphics]->data.rdp.multiUser = true;
+ gVBoxAPI.UIVRDxServer.GetNetAddress(data, VRDxServer, &netAddressUtf16);
+ if (netAddressUtf16) {
+ VBOX_UTF16_TO_UTF8(netAddressUtf16, &netAddressUtf8);
+ if (STRNEQ(netAddressUtf8, ""))
+
virDomainGraphicsListenSetAddress(def->graphics[def->ngraphics], 0,
+ netAddressUtf8, -1, true);
+ VBOX_UTF16_FREE(netAddressUtf16);
+ VBOX_UTF8_FREE(netAddressUtf8);
+ }
- gVBoxAPI.UIVRDxServer.GetReuseSingleConnection(VRDxServer,
&reuseSingleConnection);
- if (reuseSingleConnection)
- def->graphics[def->ngraphics]->data.rdp.replaceUser = true;
+ gVBoxAPI.UIVRDxServer.GetAllowMultiConnection(VRDxServer,
&allowMultiConnection);
+ if (allowMultiConnection)
+ def->graphics[def->ngraphics]->data.rdp.multiUser = true;
- def->ngraphics++;
- } else {
- virReportOOMError();
- }
+ gVBoxAPI.UIVRDxServer.GetReuseSingleConnection(VRDxServer,
&reuseSingleConnection);
+ if (reuseSingleConnection)
+ def->graphics[def->ngraphics]->data.rdp.replaceUser = true;
+
+ def->ngraphics++;
+ } else {
+ virReportOOMError();
}
- VBOX_RELEASE(VRDxServer);
}
+ VBOX_RELEASE(VRDxServer);
VBOX_UTF8_FREE(valueTypeUtf8);
}
--
2.4.10
11:02 a.m.
New subject: [libvirt] [PATCH 04/13] vboxDumpDisplay: add addDesktop bool
When FRONTEND/Type is not any of "sdl", "gui", "vrdp", we
add a DESKTOP.
Use a bool to track this, instead of checking that both
totalPresent ("sdl" or "gui" present) and vrdpPresent are zero.
---
src/vbox/vbox_common.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c
index 1c9d871..dc00a3a 100644
--- a/src/vbox/vbox_common.c
+++ b/src/vbox/vbox_common.c
@@ -3300,7 +3300,6 @@ static void
vboxDumpDisplay(virDomainDefPtr def, vboxGlobalData *data, IMachine *machine)
{
/* dump display options vrdp/gui/sdl */
- int vrdpPresent = 0;
int sdlPresent = 0;
int guiPresent = 0;
int totalPresent = 0;
@@ -3311,6 +3310,7 @@ vboxDumpDisplay(virDomainDefPtr def, vboxGlobalData *data, IMachine
*machine)
char *valueTypeUtf8 = NULL;
IVRDxServer *VRDxServer = NULL;
PRBool VRDxEnabled = PR_FALSE;
+ bool addDesktop = false;
def->ngraphics = 0;
@@ -3363,11 +3363,10 @@ vboxDumpDisplay(virDomainDefPtr def, vboxGlobalData *data,
IMachine *machine)
totalPresent++;
}
VBOX_UTF8_FREE(valueDisplayUtf8);
+ } else if (STRNEQ_NULLABLE(valueTypeUtf8, "vrdp")) {
+ addDesktop = true;
}
- if (STREQ_NULLABLE(valueTypeUtf8, "vrdp"))
- vrdpPresent = 1;
-
if ((totalPresent > 0) && (VIR_ALLOC_N(def->graphics, totalPresent)
>= 0)) {
if ((guiPresent) && (VIR_ALLOC(def->graphics[def->ngraphics]) >=
0)) {
def->graphics[def->ngraphics]->type =
VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP;
@@ -3382,7 +3381,7 @@ vboxDumpDisplay(virDomainDefPtr def, vboxGlobalData *data, IMachine
*machine)
def->graphics[def->ngraphics]->data.sdl.display = sdlDisplay;
def->ngraphics++;
}
- } else if ((vrdpPresent != 1) && (totalPresent == 0) &&
(VIR_ALLOC_N(def->graphics, 1) >= 0)) {
+ } else if (addDesktop && (VIR_ALLOC_N(def->graphics, 1) >= 0)) {
if (VIR_ALLOC(def->graphics[def->ngraphics]) >= 0) {
const char *tmp;
def->graphics[def->ngraphics]->type =
VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP;
--
2.4.10
11:02 a.m.
New subject: [libvirt] [PATCH 05/13] vboxDumpDisplay: remove extra virReportOOMError
VIR_ALLOC* already reported an error.
---
src/vbox/vbox_common.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c
index dc00a3a..54e7d47 100644
--- a/src/vbox/vbox_common.c
+++ b/src/vbox/vbox_common.c
@@ -3434,8 +3434,6 @@ vboxDumpDisplay(virDomainDefPtr def, vboxGlobalData *data, IMachine
*machine)
def->graphics[def->ngraphics]->data.rdp.replaceUser = true;
def->ngraphics++;
- } else {
- virReportOOMError();
}
}
--
2.4.10
11:02 a.m.
New subject: [libvirt] [PATCH 06/13] vboxDumpDisplay: split out def->graphics allocation
Separate allocation of the def->graphics array from the allocation
and initialization of its first element.
Note that the only possible values of totalPresent at this point
are 0 or 1, because it equals to guiPresent + sdlPresent.
---
src/vbox/vbox_common.c | 28 ++++++++++++++++++++--------
1 file changed, 20 insertions(+), 8 deletions(-)
diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c
index 54e7d47..e609e02 100644
--- a/src/vbox/vbox_common.c
+++ b/src/vbox/vbox_common.c
@@ -3296,7 +3296,7 @@ vboxDumpVideo(virDomainDefPtr def, vboxGlobalData *data
ATTRIBUTE_UNUSED,
return 0;
}
-static void
+static int
vboxDumpDisplay(virDomainDefPtr def, vboxGlobalData *data, IMachine *machine)
{
/* dump display options vrdp/gui/sdl */
@@ -3311,6 +3311,7 @@ vboxDumpDisplay(virDomainDefPtr def, vboxGlobalData *data, IMachine
*machine)
IVRDxServer *VRDxServer = NULL;
PRBool VRDxEnabled = PR_FALSE;
bool addDesktop = false;
+ int ret = -1;
def->ngraphics = 0;
@@ -3367,7 +3368,12 @@ vboxDumpDisplay(virDomainDefPtr def, vboxGlobalData *data, IMachine
*machine)
addDesktop = true;
}
- if ((totalPresent > 0) && (VIR_ALLOC_N(def->graphics, totalPresent)
>= 0)) {
+ if (totalPresent > 0 || addDesktop) {
+ if (VIR_ALLOC_N(def->graphics, 1) < 0)
+ goto cleanup;
+ }
+
+ if (totalPresent > 0) {
if ((guiPresent) && (VIR_ALLOC(def->graphics[def->ngraphics]) >=
0)) {
def->graphics[def->ngraphics]->type =
VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP;
if (guiDisplay)
@@ -3381,7 +3387,7 @@ vboxDumpDisplay(virDomainDefPtr def, vboxGlobalData *data, IMachine
*machine)
def->graphics[def->ngraphics]->data.sdl.display = sdlDisplay;
def->ngraphics++;
}
- } else if (addDesktop && (VIR_ALLOC_N(def->graphics, 1) >= 0)) {
+ } else if (addDesktop) {
if (VIR_ALLOC(def->graphics[def->ngraphics]) >= 0) {
const char *tmp;
def->graphics[def->ngraphics]->type =
VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP;
@@ -3401,11 +3407,10 @@ vboxDumpDisplay(virDomainDefPtr def, vboxGlobalData *data,
IMachine *machine)
gVBoxAPI.UIVRDxServer.GetEnabled(VRDxServer, &VRDxEnabled);
if (VRDxEnabled) {
+ if (VIR_REALLOC_N(def->graphics, totalPresent) < 0)
+ goto cleanup;
- totalPresent++;
-
- if ((VIR_REALLOC_N(def->graphics, totalPresent) >= 0) &&
- (VIR_ALLOC(def->graphics[def->ngraphics]) >= 0)) {
+ if (VIR_ALLOC(def->graphics[def->ngraphics]) >= 0) {
PRUnichar *netAddressUtf16 = NULL;
char *netAddressUtf8 = NULL;
PRBool allowMultiConnection = PR_FALSE;
@@ -3437,8 +3442,14 @@ vboxDumpDisplay(virDomainDefPtr def, vboxGlobalData *data, IMachine
*machine)
}
}
+ ret = 0;
+
+ cleanup:
+ VBOX_UTF8_FREE(guiDisplay);
+ VBOX_UTF8_FREE(sdlDisplay);
VBOX_RELEASE(VRDxServer);
VBOX_UTF8_FREE(valueTypeUtf8);
+ return ret;
}
static void
@@ -3971,7 +3982,8 @@ static char *vboxDomainGetXMLDesc(virDomainPtr dom, unsigned int
flags)
if (vboxDumpVideo(def, data, machine) < 0)
goto cleanup;
- vboxDumpDisplay(def, data, machine);
+ if (vboxDumpDisplay(def, data, machine) < 0)
+ goto cleanup;
/* As the medium interface changed from 3.0 to 3.1.
* There are two totally different implementations.
--
2.4.10
11:02 a.m.
New subject: [libvirt] [PATCH 07/13] vboxDumpDisplay: allocate the graphics structure upfront
Allocate it as soon as we know we will need it.
Add it to def->ngraphics if it's allocated, removing the need
to use the addDesktop and totalPresent variables to track this.
---
src/vbox/vbox_common.c | 77 +++++++++++++++++++++++++++-----------------------
1 file changed, 41 insertions(+), 36 deletions(-)
diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c
index e609e02..7a24de4 100644
--- a/src/vbox/vbox_common.c
+++ b/src/vbox/vbox_common.c
@@ -3302,7 +3302,6 @@ vboxDumpDisplay(virDomainDefPtr def, vboxGlobalData *data, IMachine
*machine)
/* dump display options vrdp/gui/sdl */
int sdlPresent = 0;
int guiPresent = 0;
- int totalPresent = 0;
char *guiDisplay = NULL;
char *sdlDisplay = NULL;
PRUnichar *keyTypeUtf16 = NULL;
@@ -3310,6 +3309,7 @@ vboxDumpDisplay(virDomainDefPtr def, vboxGlobalData *data, IMachine
*machine)
char *valueTypeUtf8 = NULL;
IVRDxServer *VRDxServer = NULL;
PRBool VRDxEnabled = PR_FALSE;
+ virDomainGraphicsDefPtr graphics = NULL;
bool addDesktop = false;
int ret = -1;
@@ -3330,6 +3330,9 @@ vboxDumpDisplay(virDomainDefPtr def, vboxGlobalData *data, IMachine
*machine)
PRUnichar *valueDisplayUtf16 = NULL;
char *valueDisplayUtf8 = NULL;
+ if (VIR_ALLOC(graphics) < 0)
+ goto cleanup;
+
VBOX_UTF8_TO_UTF16("FRONTEND/Display", &keyDislpayUtf16);
gVBoxAPI.UIMachine.GetExtraData(machine, keyDislpayUtf16,
&valueDisplayUtf16);
VBOX_UTF16_FREE(keyDislpayUtf16);
@@ -3350,7 +3353,6 @@ vboxDumpDisplay(virDomainDefPtr def, vboxGlobalData *data, IMachine
*machine)
* exist and then only use it there
*/
}
- totalPresent++;
}
if (STREQ(valueTypeUtf8, "gui")) {
@@ -3361,34 +3363,36 @@ vboxDumpDisplay(virDomainDefPtr def, vboxGlobalData *data,
IMachine *machine)
* exist and then only use it there
*/
}
- totalPresent++;
}
VBOX_UTF8_FREE(valueDisplayUtf8);
} else if (STRNEQ_NULLABLE(valueTypeUtf8, "vrdp")) {
- addDesktop = true;
+ if (VIR_ALLOC(graphics) < 0)
+ goto cleanup;
}
- if (totalPresent > 0 || addDesktop) {
+ if (graphics) {
if (VIR_ALLOC_N(def->graphics, 1) < 0)
goto cleanup;
+
+ def->graphics[def->ngraphics] = graphics;
+ graphics = NULL;
}
- if (totalPresent > 0) {
- if ((guiPresent) && (VIR_ALLOC(def->graphics[def->ngraphics]) >=
0)) {
+ if (guiPresent || sdlPresent) {
+ if ((guiPresent)) {
def->graphics[def->ngraphics]->type =
VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP;
if (guiDisplay)
def->graphics[def->ngraphics]->data.desktop.display =
guiDisplay;
def->ngraphics++;
}
- if ((sdlPresent) && (VIR_ALLOC(def->graphics[def->ngraphics]) >=
0)) {
+ if ((sdlPresent)) {
def->graphics[def->ngraphics]->type = VIR_DOMAIN_GRAPHICS_TYPE_SDL;
if (sdlDisplay)
def->graphics[def->ngraphics]->data.sdl.display = sdlDisplay;
def->ngraphics++;
}
} else if (addDesktop) {
- if (VIR_ALLOC(def->graphics[def->ngraphics]) >= 0) {
const char *tmp;
def->graphics[def->ngraphics]->type =
VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP;
tmp = virGetEnvBlockSUID("DISPLAY");
@@ -3397,9 +3401,7 @@ vboxDumpDisplay(virDomainDefPtr def, vboxGlobalData *data, IMachine
*machine)
* display as NULL
*/
}
- totalPresent++;
def->ngraphics++;
- }
}
gVBoxAPI.UIMachine.GetVRDxServer(machine, &VRDxServer);
@@ -3407,39 +3409,42 @@ vboxDumpDisplay(virDomainDefPtr def, vboxGlobalData *data,
IMachine *machine)
gVBoxAPI.UIVRDxServer.GetEnabled(VRDxServer, &VRDxEnabled);
if (VRDxEnabled) {
- if (VIR_REALLOC_N(def->graphics, totalPresent) < 0)
+ PRUnichar *netAddressUtf16 = NULL;
+ char *netAddressUtf8 = NULL;
+ PRBool allowMultiConnection = PR_FALSE;
+ PRBool reuseSingleConnection = PR_FALSE;
+
+ if (VIR_ALLOC(graphics) < 0)
goto cleanup;
- if (VIR_ALLOC(def->graphics[def->ngraphics]) >= 0) {
- PRUnichar *netAddressUtf16 = NULL;
- char *netAddressUtf8 = NULL;
- PRBool allowMultiConnection = PR_FALSE;
- PRBool reuseSingleConnection = PR_FALSE;
+ if (VIR_REALLOC_N(def->graphics, def->ngraphics + 1) < 0)
+ goto cleanup;
- gVBoxAPI.UIVRDxServer.GetPorts(data, VRDxServer,
def->graphics[def->ngraphics]);
+ def->graphics[def->ngraphics] = graphics;
- def->graphics[def->ngraphics]->type = VIR_DOMAIN_GRAPHICS_TYPE_RDP;
+ gVBoxAPI.UIVRDxServer.GetPorts(data, VRDxServer,
def->graphics[def->ngraphics]);
- gVBoxAPI.UIVRDxServer.GetNetAddress(data, VRDxServer, &netAddressUtf16);
- if (netAddressUtf16) {
- VBOX_UTF16_TO_UTF8(netAddressUtf16, &netAddressUtf8);
- if (STRNEQ(netAddressUtf8, ""))
-
virDomainGraphicsListenSetAddress(def->graphics[def->ngraphics], 0,
- netAddressUtf8, -1, true);
- VBOX_UTF16_FREE(netAddressUtf16);
- VBOX_UTF8_FREE(netAddressUtf8);
- }
+ def->graphics[def->ngraphics]->type = VIR_DOMAIN_GRAPHICS_TYPE_RDP;
+
+ gVBoxAPI.UIVRDxServer.GetNetAddress(data, VRDxServer, &netAddressUtf16);
+ if (netAddressUtf16) {
+ VBOX_UTF16_TO_UTF8(netAddressUtf16, &netAddressUtf8);
+ if (STRNEQ(netAddressUtf8, ""))
+ virDomainGraphicsListenSetAddress(def->graphics[def->ngraphics],
0,
+ netAddressUtf8, -1, true);
+ VBOX_UTF16_FREE(netAddressUtf16);
+ VBOX_UTF8_FREE(netAddressUtf8);
+ }
- gVBoxAPI.UIVRDxServer.GetAllowMultiConnection(VRDxServer,
&allowMultiConnection);
- if (allowMultiConnection)
- def->graphics[def->ngraphics]->data.rdp.multiUser = true;
+ gVBoxAPI.UIVRDxServer.GetAllowMultiConnection(VRDxServer,
&allowMultiConnection);
+ if (allowMultiConnection)
+ def->graphics[def->ngraphics]->data.rdp.multiUser = true;
- gVBoxAPI.UIVRDxServer.GetReuseSingleConnection(VRDxServer,
&reuseSingleConnection);
- if (reuseSingleConnection)
- def->graphics[def->ngraphics]->data.rdp.replaceUser = true;
+ gVBoxAPI.UIVRDxServer.GetReuseSingleConnection(VRDxServer,
&reuseSingleConnection);
+ if (reuseSingleConnection)
+ def->graphics[def->ngraphics]->data.rdp.replaceUser = true;
- def->ngraphics++;
- }
+ def->ngraphics++;
}
ret = 0;
--
2.4.10
11:02 a.m.
New subject: [libvirt] [PATCH 08/13] vboxDumpDisplay: fill out the graphics structure earlier
Remove the need to track what type of graphics were present
by temporary variables.
---
src/vbox/vbox_common.c | 68 +++++++++++++++++++-------------------------------
1 file changed, 25 insertions(+), 43 deletions(-)
diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c
index 7a24de4..e273de9 100644
--- a/src/vbox/vbox_common.c
+++ b/src/vbox/vbox_common.c
@@ -3300,8 +3300,6 @@ static int
vboxDumpDisplay(virDomainDefPtr def, vboxGlobalData *data, IMachine *machine)
{
/* dump display options vrdp/gui/sdl */
- int sdlPresent = 0;
- int guiPresent = 0;
char *guiDisplay = NULL;
char *sdlDisplay = NULL;
PRUnichar *keyTypeUtf16 = NULL;
@@ -3310,7 +3308,6 @@ vboxDumpDisplay(virDomainDefPtr def, vboxGlobalData *data, IMachine
*machine)
IVRDxServer *VRDxServer = NULL;
PRBool VRDxEnabled = PR_FALSE;
virDomainGraphicsDefPtr graphics = NULL;
- bool addDesktop = false;
int ret = -1;
def->ngraphics = 0;
@@ -3346,28 +3343,41 @@ vboxDumpDisplay(virDomainDefPtr def, vboxGlobalData *data,
IMachine *machine)
}
if (STREQ(valueTypeUtf8, "sdl")) {
- sdlPresent = 1;
if (VIR_STRDUP(sdlDisplay, valueDisplayUtf8) < 0) {
/* just don't go to cleanup yet as it is ok to have
* sdlDisplay as NULL and we check it below if it
* exist and then only use it there
*/
}
+ graphics->type = VIR_DOMAIN_GRAPHICS_TYPE_SDL;
+ if (sdlDisplay)
+ graphics->data.sdl.display = sdlDisplay;
}
if (STREQ(valueTypeUtf8, "gui")) {
- guiPresent = 1;
if (VIR_STRDUP(guiDisplay, valueDisplayUtf8) < 0) {
/* just don't go to cleanup yet as it is ok to have
* guiDisplay as NULL and we check it below if it
* exist and then only use it there
*/
}
+ graphics->type = VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP;
+ if (guiDisplay)
+ graphics->data.desktop.display = guiDisplay;
}
VBOX_UTF8_FREE(valueDisplayUtf8);
} else if (STRNEQ_NULLABLE(valueTypeUtf8, "vrdp")) {
+ const char *tmp;
if (VIR_ALLOC(graphics) < 0)
goto cleanup;
+
+ graphics->type = VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP;
+ tmp = virGetEnvBlockSUID("DISPLAY");
+ if (VIR_STRDUP(graphics->data.desktop.display, tmp) < 0) {
+ /* just don't go to cleanup yet as it is ok to have
+ * display as NULL
+ */
+ }
}
if (graphics) {
@@ -3376,32 +3386,7 @@ vboxDumpDisplay(virDomainDefPtr def, vboxGlobalData *data, IMachine
*machine)
def->graphics[def->ngraphics] = graphics;
graphics = NULL;
- }
-
- if (guiPresent || sdlPresent) {
- if ((guiPresent)) {
- def->graphics[def->ngraphics]->type =
VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP;
- if (guiDisplay)
- def->graphics[def->ngraphics]->data.desktop.display =
guiDisplay;
- def->ngraphics++;
- }
-
- if ((sdlPresent)) {
- def->graphics[def->ngraphics]->type = VIR_DOMAIN_GRAPHICS_TYPE_SDL;
- if (sdlDisplay)
- def->graphics[def->ngraphics]->data.sdl.display = sdlDisplay;
- def->ngraphics++;
- }
- } else if (addDesktop) {
- const char *tmp;
- def->graphics[def->ngraphics]->type =
VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP;
- tmp = virGetEnvBlockSUID("DISPLAY");
- if (VIR_STRDUP(def->graphics[def->ngraphics]->data.desktop.display,
tmp) < 0) {
- /* just don't go to cleanup yet as it is ok to have
- * display as NULL
- */
- }
- def->ngraphics++;
+ def->ngraphics++;
}
gVBoxAPI.UIMachine.GetVRDxServer(machine, &VRDxServer);
@@ -3417,20 +3402,15 @@ vboxDumpDisplay(virDomainDefPtr def, vboxGlobalData *data,
IMachine *machine)
if (VIR_ALLOC(graphics) < 0)
goto cleanup;
- if (VIR_REALLOC_N(def->graphics, def->ngraphics + 1) < 0)
- goto cleanup;
+ gVBoxAPI.UIVRDxServer.GetPorts(data, VRDxServer, graphics);
- def->graphics[def->ngraphics] = graphics;
-
- gVBoxAPI.UIVRDxServer.GetPorts(data, VRDxServer,
def->graphics[def->ngraphics]);
-
- def->graphics[def->ngraphics]->type = VIR_DOMAIN_GRAPHICS_TYPE_RDP;
+ graphics->type = VIR_DOMAIN_GRAPHICS_TYPE_RDP;
gVBoxAPI.UIVRDxServer.GetNetAddress(data, VRDxServer, &netAddressUtf16);
if (netAddressUtf16) {
VBOX_UTF16_TO_UTF8(netAddressUtf16, &netAddressUtf8);
if (STRNEQ(netAddressUtf8, ""))
- virDomainGraphicsListenSetAddress(def->graphics[def->ngraphics],
0,
+ virDomainGraphicsListenSetAddress(graphics, 0,
netAddressUtf8, -1, true);
VBOX_UTF16_FREE(netAddressUtf16);
VBOX_UTF8_FREE(netAddressUtf8);
@@ -3438,20 +3418,22 @@ vboxDumpDisplay(virDomainDefPtr def, vboxGlobalData *data,
IMachine *machine)
gVBoxAPI.UIVRDxServer.GetAllowMultiConnection(VRDxServer,
&allowMultiConnection);
if (allowMultiConnection)
- def->graphics[def->ngraphics]->data.rdp.multiUser = true;
+ graphics->data.rdp.multiUser = true;
gVBoxAPI.UIVRDxServer.GetReuseSingleConnection(VRDxServer,
&reuseSingleConnection);
if (reuseSingleConnection)
- def->graphics[def->ngraphics]->data.rdp.replaceUser = true;
+ graphics->data.rdp.replaceUser = true;
+
+ if (VIR_REALLOC_N(def->graphics, def->ngraphics + 1) < 0)
+ goto cleanup;
+ def->graphics[def->ngraphics] = graphics;
def->ngraphics++;
}
ret = 0;
cleanup:
- VBOX_UTF8_FREE(guiDisplay);
- VBOX_UTF8_FREE(sdlDisplay);
VBOX_RELEASE(VRDxServer);
VBOX_UTF8_FREE(valueTypeUtf8);
return ret;
--
2.4.10
11:02 a.m.
New subject: [libvirt] [PATCH 09/13] vboxDumpDisplay: clean up VIR_STRDUP usage
Two VIR_STRDUP calls are redundant - just steal the string
converted by VBOX_UTF16_TO_UTF8.
Report an error when the third one fails.
---
src/vbox/vbox_common.c | 32 +++++++-------------------------
1 file changed, 7 insertions(+), 25 deletions(-)
diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c
index e273de9..43de343 100644
--- a/src/vbox/vbox_common.c
+++ b/src/vbox/vbox_common.c
@@ -3300,8 +3300,6 @@ static int
vboxDumpDisplay(virDomainDefPtr def, vboxGlobalData *data, IMachine *machine)
{
/* dump display options vrdp/gui/sdl */
- char *guiDisplay = NULL;
- char *sdlDisplay = NULL;
PRUnichar *keyTypeUtf16 = NULL;
PRUnichar *valueTypeUtf16 = NULL;
char *valueTypeUtf8 = NULL;
@@ -3343,41 +3341,25 @@ vboxDumpDisplay(virDomainDefPtr def, vboxGlobalData *data,
IMachine *machine)
}
if (STREQ(valueTypeUtf8, "sdl")) {
- if (VIR_STRDUP(sdlDisplay, valueDisplayUtf8) < 0) {
- /* just don't go to cleanup yet as it is ok to have
- * sdlDisplay as NULL and we check it below if it
- * exist and then only use it there
- */
- }
graphics->type = VIR_DOMAIN_GRAPHICS_TYPE_SDL;
- if (sdlDisplay)
- graphics->data.sdl.display = sdlDisplay;
+ graphics->data.sdl.display = valueDisplayUtf8;
+ valueDisplayUtf8 = NULL;
}
if (STREQ(valueTypeUtf8, "gui")) {
- if (VIR_STRDUP(guiDisplay, valueDisplayUtf8) < 0) {
- /* just don't go to cleanup yet as it is ok to have
- * guiDisplay as NULL and we check it below if it
- * exist and then only use it there
- */
- }
graphics->type = VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP;
- if (guiDisplay)
- graphics->data.desktop.display = guiDisplay;
+ graphics->data.desktop.display = valueDisplayUtf8;
+ valueDisplayUtf8 = NULL;
}
VBOX_UTF8_FREE(valueDisplayUtf8);
} else if (STRNEQ_NULLABLE(valueTypeUtf8, "vrdp")) {
- const char *tmp;
if (VIR_ALLOC(graphics) < 0)
goto cleanup;
graphics->type = VIR_DOMAIN_GRAPHICS_TYPE_DESKTOP;
- tmp = virGetEnvBlockSUID("DISPLAY");
- if (VIR_STRDUP(graphics->data.desktop.display, tmp) < 0) {
- /* just don't go to cleanup yet as it is ok to have
- * display as NULL
- */
- }
+ if (VIR_STRDUP(graphics->data.desktop.display,
+ virGetEnvBlockSUID("DISPLAY")) < 0)
+ goto cleanup;
}
if (graphics) {
--
2.4.10
11:02 a.m.
New subject: [libvirt] [PATCH 10/13] vboxDumpDisplay: check return of virDomainGraphicsListenSetAddress
Error out if the allocation failed.
---
src/vbox/vbox_common.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c
index 43de343..1c2a432 100644
--- a/src/vbox/vbox_common.c
+++ b/src/vbox/vbox_common.c
@@ -3303,6 +3303,7 @@ vboxDumpDisplay(virDomainDefPtr def, vboxGlobalData *data, IMachine
*machine)
PRUnichar *keyTypeUtf16 = NULL;
PRUnichar *valueTypeUtf16 = NULL;
char *valueTypeUtf8 = NULL;
+ char *netAddressUtf8 = NULL;
IVRDxServer *VRDxServer = NULL;
PRBool VRDxEnabled = PR_FALSE;
virDomainGraphicsDefPtr graphics = NULL;
@@ -3377,7 +3378,6 @@ vboxDumpDisplay(virDomainDefPtr def, vboxGlobalData *data, IMachine
*machine)
if (VRDxEnabled) {
PRUnichar *netAddressUtf16 = NULL;
- char *netAddressUtf8 = NULL;
PRBool allowMultiConnection = PR_FALSE;
PRBool reuseSingleConnection = PR_FALSE;
@@ -3391,13 +3391,14 @@ vboxDumpDisplay(virDomainDefPtr def, vboxGlobalData *data,
IMachine *machine)
gVBoxAPI.UIVRDxServer.GetNetAddress(data, VRDxServer, &netAddressUtf16);
if (netAddressUtf16) {
VBOX_UTF16_TO_UTF8(netAddressUtf16, &netAddressUtf8);
- if (STRNEQ(netAddressUtf8, ""))
- virDomainGraphicsListenSetAddress(graphics, 0,
- netAddressUtf8, -1, true);
VBOX_UTF16_FREE(netAddressUtf16);
- VBOX_UTF8_FREE(netAddressUtf8);
}
+ if (STRNEQ_NULLABLE(netAddressUtf8, "") &&
+ virDomainGraphicsListenSetAddress(graphics, 0,
+ netAddressUtf8, -1, true) < 0)
+ goto cleanup;
+
gVBoxAPI.UIVRDxServer.GetAllowMultiConnection(VRDxServer,
&allowMultiConnection);
if (allowMultiConnection)
graphics->data.rdp.multiUser = true;
@@ -3418,6 +3419,7 @@ vboxDumpDisplay(virDomainDefPtr def, vboxGlobalData *data, IMachine
*machine)
cleanup:
VBOX_RELEASE(VRDxServer);
VBOX_UTF8_FREE(valueTypeUtf8);
+ VBOX_UTF8_FREE(netAddressUtf8);
return ret;
}
--
2.4.10
11:02 a.m.
New subject: [libvirt] [PATCH 11/13] vboxDumpDisplay: use VIR_APPEND_ELEMENT
Instead of open-coding it.
---
src/vbox/vbox_common.c | 16 ++++------------
1 file changed, 4 insertions(+), 12 deletions(-)
diff --git a/src/vbox/vbox_common.c b/src/vbox/vbox_common.c
index 1c2a432..236e048 100644
--- a/src/vbox/vbox_common.c
+++ b/src/vbox/vbox_common.c
@@ -3363,14 +3363,9 @@ vboxDumpDisplay(virDomainDefPtr def, vboxGlobalData *data, IMachine
*machine)
goto cleanup;
}
- if (graphics) {
- if (VIR_ALLOC_N(def->graphics, 1) < 0)
- goto cleanup;
-
- def->graphics[def->ngraphics] = graphics;
- graphics = NULL;
- def->ngraphics++;
- }
+ if (graphics &&
+ VIR_APPEND_ELEMENT(def->graphics, def->ngraphics, graphics) < 0)
+ goto cleanup;
gVBoxAPI.UIMachine.GetVRDxServer(machine, &VRDxServer);
if (VRDxServer)
@@ -3407,11 +3402,8 @@ vboxDumpDisplay(virDomainDefPtr def, vboxGlobalData *data, IMachine
*machine)
if (reuseSingleConnection)
graphics->data.rdp.replaceUser = true;
- if (VIR_REALLOC_N(def->graphics, def->ngraphics + 1) < 0)
+ if (VIR_APPEND_ELEMENT(def->graphics, def->ngraphics, graphics) < 0)
goto cleanup;
-
- def->graphics[def->ngraphics] = graphics;
- def->ngraphics++;
}
ret = 0;
--
2.4.10
11:02 a.m.
New subject: [libvirt] [PATCH 12/13] vboxDumpDisplay: reuse the keyUtf16 variable
We free the key right after calling the API.
Reuse a single variable to remove the typo.
---
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 236e048..71fc62b 100644
--- a/src/vbox/vbox_common.c
+++ b/src/vbox/vbox_common.c
@@ -3300,7 +3300,7 @@ static int
vboxDumpDisplay(virDomainDefPtr def, vboxGlobalData *data, IMachine *machine)
{
/* dump display options vrdp/gui/sdl */
- PRUnichar *keyTypeUtf16 = NULL;
+ PRUnichar *keyUtf16 = NULL;
PRUnichar *valueTypeUtf16 = NULL;
char *valueTypeUtf8 = NULL;
char *netAddressUtf8 = NULL;
@@ -3311,9 +3311,9 @@ vboxDumpDisplay(virDomainDefPtr def, vboxGlobalData *data, IMachine
*machine)
def->ngraphics = 0;
- VBOX_UTF8_TO_UTF16("FRONTEND/Type", &keyTypeUtf16);
- gVBoxAPI.UIMachine.GetExtraData(machine, keyTypeUtf16, &valueTypeUtf16);
- VBOX_UTF16_FREE(keyTypeUtf16);
+ VBOX_UTF8_TO_UTF16("FRONTEND/Type", &keyUtf16);
+ gVBoxAPI.UIMachine.GetExtraData(machine, keyUtf16, &valueTypeUtf16);
+ VBOX_UTF16_FREE(keyUtf16);
if (valueTypeUtf16) {
VBOX_UTF16_TO_UTF8(valueTypeUtf16, &valueTypeUtf8);
@@ -3322,16 +3322,15 @@ vboxDumpDisplay(virDomainDefPtr def, vboxGlobalData *data,
IMachine *machine)
if (STREQ_NULLABLE(valueTypeUtf8, "sdl") ||
STREQ_NULLABLE(valueTypeUtf8, "gui")) {
- PRUnichar *keyDislpayUtf16 = NULL;
PRUnichar *valueDisplayUtf16 = NULL;
char *valueDisplayUtf8 = NULL;
if (VIR_ALLOC(graphics) < 0)
goto cleanup;
- VBOX_UTF8_TO_UTF16("FRONTEND/Display", &keyDislpayUtf16);
- gVBoxAPI.UIMachine.GetExtraData(machine, keyDislpayUtf16,
&valueDisplayUtf16);
- VBOX_UTF16_FREE(keyDislpayUtf16);
+ VBOX_UTF8_TO_UTF16("FRONTEND/Display", &keyUtf16);
+ gVBoxAPI.UIMachine.GetExtraData(machine, keyUtf16, &valueDisplayUtf16);
+ VBOX_UTF16_FREE(keyUtf16);
if (valueDisplayUtf16) {
VBOX_UTF16_TO_UTF8(valueDisplayUtf16, &valueDisplayUtf8);
--
2.4.10
11:02 a.m.
New subject: [libvirt] [PATCH 13/13] vboxDumpDisplay: remove suspicious strlen
The return type of strlen is 'size_t', which is unsigned and therefore
never less than zero.
Use STREQ to make the check obvious.
---
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 71fc62b..0c0dc96 100644
--- a/src/vbox/vbox_common.c
+++ b/src/vbox/vbox_common.c
@@ -3336,7 +3336,7 @@ vboxDumpDisplay(virDomainDefPtr def, vboxGlobalData *data, IMachine
*machine)
VBOX_UTF16_TO_UTF8(valueDisplayUtf16, &valueDisplayUtf8);
VBOX_UTF16_FREE(valueDisplayUtf16);
- if (strlen(valueDisplayUtf8) <= 0)
+ if (STREQ(valueDisplayUtf8, ""))
VBOX_UTF8_FREE(valueDisplayUtf8);
}
--
2.4.10
10:06 a.m.
On 05.02.2016 18:02, Ján Tomko wrote:
Check for return values of libvirt's internal APIs in
vboxDumpVideo
and vboxDumpDisplay.
Patch 10/13 should fix a complaint by Coverity (untested).
Lots of whitespace changes, -b is your friend.
Ján Tomko (13):
Check return value of vboxDumpVideo
vboxDumpDisplay: reduce indentation level
vboxDumpDisplay: more indentation reducing
vboxDumpDisplay: add addDesktop bool
vboxDumpDisplay: remove extra virReportOOMError
vboxDumpDisplay: split out def->graphics allocation
vboxDumpDisplay: allocate the graphics structure upfront
vboxDumpDisplay: fill out the graphics structure earlier
vboxDumpDisplay: clean up VIR_STRDUP usage
vboxDumpDisplay: check return of virDomainGraphicsListenSetAddress
vboxDumpDisplay: use VIR_APPEND_ELEMENT
vboxDumpDisplay: reuse the keyUtf16 variable
vboxDumpDisplay: remove suspicious strlen
src/vbox/vbox_common.c | 249 ++++++++++++++++++++++---------------------------
1 file changed, 113 insertions(+), 136 deletions(-)
ACK series. Any intention to continue or is this just a one timer?
Michal
3196
days inactive
3199
days old
15 comments
2 participants
participants (2)
-
Ján Tomko -
Michal Privoznik