[libvirt] [PATCH] conf: fix no error when set an unsupport string in ./devices/shmem/msi[@ioeventfd]
https://bugzilla.redhat.com/show_bug.cgi?id=1220265 Pass the return value to an enum directly is not safe. When pass a invalid @ioeventfd and virTristateSwitchTypeFromString return -1 to def->msi.ioeventfd, and this value transform to 4294967295, so no error when the parse failed. Instead of define a int variable in virDomainShmemDefParseXML and use the new defined variable as transition variable. I think define ioeventfd as an 'int' variable will be better. Signed-off-by: Luyao Huang <lhuang@redhat.com> --- src/conf/domain_conf.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 087d282..fea9baa 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1610,7 +1610,7 @@ struct _virDomainShmemDef { struct { bool enabled; unsigned vectors; - virTristateSwitch ioeventfd; + int ioeventfd; /* enum virTristateSwitch */ } msi; virDomainDeviceInfo info; }; -- 1.8.3.1
On Mon, May 11, 2015 at 04:03:19PM +0800, Luyao Huang wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1220265
Pass the return value to an enum directly is not safe. When pass a invalid @ioeventfd and virTristateSwitchTypeFromString return -1 to def->msi.ioeventfd, and this value transform to 4294967295, so no error when the parse failed.
Instead of define a int variable in virDomainShmemDefParseXML and use the new defined variable as transition variable. I think define ioeventfd as an 'int' variable will be better.
Signed-off-by: Luyao Huang <lhuang@redhat.com> --- src/conf/domain_conf.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 087d282..fea9baa 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1610,7 +1610,7 @@ struct _virDomainShmemDef { struct { bool enabled; unsigned vectors; - virTristateSwitch ioeventfd; + int ioeventfd; /* enum virTristateSwitch */ } msi; virDomainDeviceInfo info; }; -- 1.8.3.1
Good catch, but it's cleaner to folter the value using int and then assign it to virTristateSwitch as it's done e.g. with ioeventfd in virDomainDiskDefParseXML().
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
On 05/11/2015 05:40 PM, Martin Kletzander wrote:
On Mon, May 11, 2015 at 04:03:19PM +0800, Luyao Huang wrote:
https://bugzilla.redhat.com/show_bug.cgi?id=1220265
Pass the return value to an enum directly is not safe. When pass a invalid @ioeventfd and virTristateSwitchTypeFromString return -1 to def->msi.ioeventfd, and this value transform to 4294967295, so no error when the parse failed.
Instead of define a int variable in virDomainShmemDefParseXML and use the new defined variable as transition variable. I think define ioeventfd as an 'int' variable will be better.
Signed-off-by: Luyao Huang <lhuang@redhat.com> --- src/conf/domain_conf.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 087d282..fea9baa 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -1610,7 +1610,7 @@ struct _virDomainShmemDef { struct { bool enabled; unsigned vectors; - virTristateSwitch ioeventfd; + int ioeventfd; /* enum virTristateSwitch */ } msi; virDomainDeviceInfo info; }; -- 1.8.3.1
Good catch, but it's cleaner to folter the value using int and then assign it to virTristateSwitch as it's done e.g. with ioeventfd in virDomainDiskDefParseXML().
Okay, I will use another way to make code more clearly. Thanks for your quick review Luyao
participants (2)
-
Luyao Huang -
Martin Kletzander