7:10 a.m.
On 25.11.2015 09:42, Marc-André Lureau wrote:
Allowing to have the extra undefined/default state.
Signed-off-by: Marc-André Lureau <marcandre.lureau(a)gmail.com>
---
src/conf/domain_conf.c | 41 ++++++++++++++++++++++++++---------------
src/conf/domain_conf.h | 4 ++--
src/vbox/vbox_common.c | 18 ++++++++++++------
3 files changed, 40 insertions(+), 23 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index f501f14..111c2ae 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -11961,8 +11961,9 @@ virDomainVideoAccelDefParseXML(xmlNodePtr node)
{
xmlNodePtr cur;
virDomainVideoAccelDefPtr def;
- char *accel3d = NULL;
char *accel2d = NULL;
+ char *accel3d = NULL;
+ int val;
This @val variable is not much useful, since def->accel{2,3}d is already type of int,
but doesn't hurt to have it here too. I bet compiler will optimize it out anyway. And
I don't want to be too picky too.
cur = node->children;
while (cur != NULL) {
@@ -11983,21 +11984,26 @@ virDomainVideoAccelDefParseXML(xmlNodePtr node)
return NULL;
There's another bug in here, just above this return NULL ^^^ there are two
allocations, if the first succeeds but the second fails, we leak the first allocation. So
this return NULL should be replaced with goto cleanup.
if (accel3d) {
- if (STREQ(accel3d, "yes"))
- def->accel3d = true;
- else
- def->accel3d = false;
- VIR_FREE(accel3d);
+ if ((val = virTristateBoolTypeFromString(accel3d)) <= 0) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unknown accel3d value '%s'"), accel3d);
+ goto end;
+ }
+ def->accel3d = val;
}
if (accel2d) {
- if (STREQ(accel2d, "yes"))
- def->accel2d = true;
- else
- def->accel2d = false;
- VIR_FREE(accel2d);
+ if ((val = virTristateBoolTypeFromString(accel2d)) <= 0) {
+ virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+ _("unknown accel2d value '%s'"), accel2d);
+ goto end;
+ }
+ def->accel2d = val;
}
+end:
s/^/ / it's our coding style. Then, we tend to use cleanup as a name for labels rather
than end.
+ VIR_FREE(accel2d);
+ VIR_FREE(accel3d);
return def;
}
@@ -20837,10 +20843,15 @@ static void
virDomainVideoAccelDefFormat(virBufferPtr buf,
virDomainVideoAccelDefPtr def)
{
- virBufferAsprintf(buf, "<acceleration accel3d='%s'",
- def->accel3d ? "yes" : "no");
- virBufferAsprintf(buf, " accel2d='%s'",
- def->accel2d ? "yes" : "no");
+ virBufferAsprintf(buf, "<acceleration");
For adding literal strings to a buffer we have a special function: virBufferAddLit().
+ if (def->accel3d) {
+ virBufferAsprintf(buf, " accel3d='%s'",
+ virTristateBoolTypeToString(def->accel3d));
+ }
+ if (def->accel2d) {
+ virBufferAsprintf(buf, " accel2d='%s'",
+ virTristateBoolTypeToString(def->accel2d));
+ }
virBufferAddLit(buf, "/>\n");
}
Right, I went through all occurrences of accel{2,3}d and you are fixing all the places
needed.
ACK with this squashed in:
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index e095115..e04227a 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -12027,13 +12027,13 @@ virDomainVideoAccelDefParseXML(xmlNodePtr node)
return NULL;
if (VIR_ALLOC(def) < 0)
- return NULL;
+ goto cleanup;
if (accel3d) {
if ((val = virTristateBoolTypeFromString(accel3d)) <= 0) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("unknown accel3d value '%s'"), accel3d);
- goto end;
+ goto cleanup;
}
def->accel3d = val;
}
@@ -12042,12 +12042,12 @@ virDomainVideoAccelDefParseXML(xmlNodePtr node)
if ((val = virTristateBoolTypeFromString(accel2d)) <= 0) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("unknown accel2d value '%s'"), accel2d);
- goto end;
+ goto cleanup;
}
def->accel2d = val;
}
-end:
+ cleanup:
VIR_FREE(accel2d);
VIR_FREE(accel3d);
return def;
@@ -20879,7 +20879,7 @@ static void
virDomainVideoAccelDefFormat(virBufferPtr buf,
virDomainVideoAccelDefPtr def)
{
- virBufferAsprintf(buf, "<acceleration");
+ virBufferAddLit(buf, "<acceleration");
if (def->accel3d) {
virBufferAsprintf(buf, " accel3d='%s'",
virTristateBoolTypeToString(def->accel3d));
Michal