[libvirt] [PATCH] conf: report error in virCPUDefParseXML
When detected invalid 'memAccess', virCPUDefParseXML should report error. Resolves https://bugzilla.redhat.com/show_bug.cgi?id=1146334 Signed-off-by: Jincheng Miao <jmiao@redhat.com> --- src/conf/cpu_conf.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c index 116aa58..a1081b9 100644 --- a/src/conf/cpu_conf.c +++ b/src/conf/cpu_conf.c @@ -510,13 +510,13 @@ virCPUDefParseXML(xmlNodePtr node, def->cells[cur_cell].memAccess = virMemAccessTypeFromString(memAccessStr); - if (def->cells[cur_cell].memAccess <= 0) { + if ((int)def->cells[cur_cell].memAccess <= 0) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Invalid 'memAccess' attribute " "value '%s'"), memAccessStr); VIR_FREE(memAccessStr); - goto cleanup; + goto error; } VIR_FREE(memAccessStr); } -- 1.9.3
On Thu, Sep 25, 2014 at 12:23:31PM +0800, Jincheng Miao wrote:
When detected invalid 'memAccess', virCPUDefParseXML should report error.
Resolves https://bugzilla.redhat.com/show_bug.cgi?id=1146334
Signed-off-by: Jincheng Miao <jmiao@redhat.com> --- src/conf/cpu_conf.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c index 116aa58..a1081b9 100644 --- a/src/conf/cpu_conf.c +++ b/src/conf/cpu_conf.c @@ -510,13 +510,13 @@ virCPUDefParseXML(xmlNodePtr node, def->cells[cur_cell].memAccess = virMemAccessTypeFromString(memAccessStr);
- if (def->cells[cur_cell].memAccess <= 0) { + if ((int)def->cells[cur_cell].memAccess <= 0) {
Any reason for the explicit cast in here?
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Invalid 'memAccess' attribute " "value '%s'"), memAccessStr); VIR_FREE(memAccessStr); - goto cleanup; + goto error;
ACK to this line. Martin
On 09/25/2014 01:48 PM, Martin Kletzander wrote:
On Thu, Sep 25, 2014 at 12:23:31PM +0800, Jincheng Miao wrote:
When detected invalid 'memAccess', virCPUDefParseXML should report error.
Resolves https://bugzilla.redhat.com/show_bug.cgi?id=1146334
Signed-off-by: Jincheng Miao <jmiao@redhat.com> --- src/conf/cpu_conf.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c index 116aa58..a1081b9 100644 --- a/src/conf/cpu_conf.c +++ b/src/conf/cpu_conf.c @@ -510,13 +510,13 @@ virCPUDefParseXML(xmlNodePtr node, def->cells[cur_cell].memAccess = virMemAccessTypeFromString(memAccessStr);
- if (def->cells[cur_cell].memAccess <= 0) { + if ((int)def->cells[cur_cell].memAccess <= 0) {
Any reason for the explicit cast in here?
Yes, I found enum virMemAccess will be convert to uint, like: (gdb) c Continuing. Breakpoint 2, virCPUDefParseXML (node=<optimized out>, ctxt=ctxt@entry=0x7f92e80034a0, mode=mode@entry=VIR_CPU_TYPE_GUEST) at conf/cpu_conf.c:513 513 if (def->cells[cur_cell].memAccess <= 0) { (gdb) whatis def->cells[cur_cell].memAccess type = virMemAccess (gdb) p def->cells[cur_cell].memAccess $1 = 4294967295 (gdb) p def->cells[cur_cell].memAccess <= 0 $2 = 0
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("Invalid 'memAccess' attribute " "value '%s'"), memAccessStr); VIR_FREE(memAccessStr); - goto cleanup; + goto error;
ACK to this line.
Martin
On 09/25/14 09:25, Jincheng Miao wrote:
On 09/25/2014 01:48 PM, Martin Kletzander wrote:
On Thu, Sep 25, 2014 at 12:23:31PM +0800, Jincheng Miao wrote:
When detected invalid 'memAccess', virCPUDefParseXML should report error.
Resolves https://bugzilla.redhat.com/show_bug.cgi?id=1146334
Signed-off-by: Jincheng Miao <jmiao@redhat.com> --- src/conf/cpu_conf.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c index 116aa58..a1081b9 100644 --- a/src/conf/cpu_conf.c +++ b/src/conf/cpu_conf.c @@ -510,13 +510,13 @@ virCPUDefParseXML(xmlNodePtr node, def->cells[cur_cell].memAccess = virMemAccessTypeFromString(memAccessStr);
- if (def->cells[cur_cell].memAccess <= 0) { + if ((int)def->cells[cur_cell].memAccess <= 0) {
Any reason for the explicit cast in here?
Yes, I found enum virMemAccess will be convert to uint, like:
(gdb) c Continuing.
Breakpoint 2, virCPUDefParseXML (node=<optimized out>, ctxt=ctxt@entry=0x7f92e80034a0, mode=mode@entry=VIR_CPU_TYPE_GUEST) at conf/cpu_conf.c:513 513 if (def->cells[cur_cell].memAccess <= 0) { (gdb) whatis def->cells[cur_cell].memAccess type = virMemAccess (gdb) p def->cells[cur_cell].memAccess $1 = 4294967295 (gdb) p def->cells[cur_cell].memAccess <= 0 $2 = 0
In that case, use an intermediate variable with the correct type and assign it only after you verify that the value is correct. Peter
On 09/25/2014 04:06 PM, Peter Krempa wrote:
On 09/25/14 09:25, Jincheng Miao wrote:
On 09/25/2014 01:48 PM, Martin Kletzander wrote:
On Thu, Sep 25, 2014 at 12:23:31PM +0800, Jincheng Miao wrote:
When detected invalid 'memAccess', virCPUDefParseXML should report error.
Resolves https://bugzilla.redhat.com/show_bug.cgi?id=1146334
Signed-off-by: Jincheng Miao <jmiao@redhat.com> --- src/conf/cpu_conf.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/conf/cpu_conf.c b/src/conf/cpu_conf.c index 116aa58..a1081b9 100644 --- a/src/conf/cpu_conf.c +++ b/src/conf/cpu_conf.c @@ -510,13 +510,13 @@ virCPUDefParseXML(xmlNodePtr node, def->cells[cur_cell].memAccess = virMemAccessTypeFromString(memAccessStr);
- if (def->cells[cur_cell].memAccess <= 0) { + if ((int)def->cells[cur_cell].memAccess <= 0) { Any reason for the explicit cast in here? Yes, I found enum virMemAccess will be convert to uint, like:
(gdb) c Continuing.
Breakpoint 2, virCPUDefParseXML (node=<optimized out>, ctxt=ctxt@entry=0x7f92e80034a0, mode=mode@entry=VIR_CPU_TYPE_GUEST) at conf/cpu_conf.c:513 513 if (def->cells[cur_cell].memAccess <= 0) { (gdb) whatis def->cells[cur_cell].memAccess type = virMemAccess (gdb) p def->cells[cur_cell].memAccess $1 = 4294967295 (gdb) p def->cells[cur_cell].memAccess <= 0 $2 = 0
In that case, use an intermediate variable with the correct type and assign it only after you verify that the value is correct.
Yes, you are right, it nonsense to set a unverified value to vm->def. Version 2 will come up. Thanks Jincheng Miao
Peter
participants (3)
-
Jincheng Miao -
Martin Kletzander -
Peter Krempa