[libvirt] [PATCH] network: check negative values in bridge queues
We already are checking for negative value, reporting an error, but using wrong function, so the check never succeeds. This patch provides just a minor change in call of the right version of function virStrToLong. Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1138539 --- src/conf/domain_conf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index a2a7d92..42b8973 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -7374,7 +7374,7 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, } if (queues) { unsigned int q; - if (virStrToLong_ui(queues, NULL, 10, &q) < 0) { + if (virStrToLong_uip(queues, NULL, 10, &q) < 0) { virReportError(VIR_ERR_XML_DETAIL, _("'queues' attribute must be positive number: %s"), queues); -- 1.9.3
On Mon, Sep 15, 2014 at 10:38:20AM +0200, Erik Skultety wrote:
We already are checking for negative value, reporting an error, but using wrong function, so the check never succeeds. This patch provides
Actually it could succeed, for example with queues="asdf" or "3d" it would return a -1. So I reworded it a bit.
just a minor change in call of the right version of function virStrToLong.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1138539 --- src/conf/domain_conf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index a2a7d92..42b8973 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -7374,7 +7374,7 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, } if (queues) { unsigned int q; - if (virStrToLong_ui(queues, NULL, 10, &q) < 0) { + if (virStrToLong_uip(queues, NULL, 10, &q) < 0) { virReportError(VIR_ERR_XML_DETAIL, _("'queues' attribute must be positive number: %s"), queues); -- 1.9.3
ACK && Pushed, Martin
On Mon, Sep 15, 2014 at 05:34:36PM +0200, Martin Kletzander wrote:
On Mon, Sep 15, 2014 at 10:38:20AM +0200, Erik Skultety wrote:
We already are checking for negative value, reporting an error, but using wrong function, so the check never succeeds. This patch provides
Actually it could succeed, for example with queues="asdf" or "3d" it would return a -1. So I reworded it a bit.
just a minor change in call of the right version of function virStrToLong.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1138539 --- src/conf/domain_conf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index a2a7d92..42b8973 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -7374,7 +7374,7 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, } if (queues) { unsigned int q; - if (virStrToLong_ui(queues, NULL, 10, &q) < 0) { + if (virStrToLong_uip(queues, NULL, 10, &q) < 0) { virReportError(VIR_ERR_XML_DETAIL, _("'queues' attribute must be positive number: %s"), queues); -- 1.9.3
ACK && Pushed,
Well, I should've said that having a test for that can show that it works properly. It should be named *-invalid.xml and that will make schema tests check whether the RNG schema fits as well. That would be a great follow-up! ;-)
Martin
On Mon, Sep 15, 2014 at 05:41:38PM +0200, Martin Kletzander wrote:
On Mon, Sep 15, 2014 at 05:34:36PM +0200, Martin Kletzander wrote:
On Mon, Sep 15, 2014 at 10:38:20AM +0200, Erik Skultety wrote:
We already are checking for negative value, reporting an error, but using wrong function, so the check never succeeds. This patch provides
Actually it could succeed, for example with queues="asdf" or "3d" it would return a -1. So I reworded it a bit.
just a minor change in call of the right version of function virStrToLong.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1138539 --- src/conf/domain_conf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index a2a7d92..42b8973 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -7374,7 +7374,7 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, } if (queues) { unsigned int q; - if (virStrToLong_ui(queues, NULL, 10, &q) < 0) { + if (virStrToLong_uip(queues, NULL, 10, &q) < 0) { virReportError(VIR_ERR_XML_DETAIL, _("'queues' attribute must be positive number: %s"), queues); -- 1.9.3
ACK && Pushed,
Well, I should've said that having a test for that can show that it works properly. It should be named *-invalid.xml and that will make schema tests check whether the RNG schema fits as well. That would be a great follow-up! ;-)
Sorry for spamming, but I just realized that I haven't pushed it yet, so I'm reverting my ACK and please add the tests and make sure that RNG schema fits. Thanks, Martin
Thanks for review. Erik On 09/15/2014 05:51 PM, Martin Kletzander wrote:
On Mon, Sep 15, 2014 at 05:41:38PM +0200, Martin Kletzander wrote:
On Mon, Sep 15, 2014 at 05:34:36PM +0200, Martin Kletzander wrote:
On Mon, Sep 15, 2014 at 10:38:20AM +0200, Erik Skultety wrote:
We already are checking for negative value, reporting an error, but using wrong function, so the check never succeeds. This patch provides
Actually it could succeed, for example with queues="asdf" or "3d" it would return a -1. So I reworded it a bit.
just a minor change in call of the right version of function virStrToLong.
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1138539 --- src/conf/domain_conf.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index a2a7d92..42b8973 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -7374,7 +7374,7 @@ virDomainNetDefParseXML(virDomainXMLOptionPtr xmlopt, } if (queues) { unsigned int q; - if (virStrToLong_ui(queues, NULL, 10, &q) < 0) { + if (virStrToLong_uip(queues, NULL, 10, &q) < 0) { virReportError(VIR_ERR_XML_DETAIL, _("'queues' attribute must be positive number: %s"), queues); -- 1.9.3
ACK && Pushed,
Well, I should've said that having a test for that can show that it works properly. It should be named *-invalid.xml and that will make schema tests check whether the RNG schema fits as well. That would be a great follow-up! ;-)
Sorry for spamming, but I just realized that I haven't pushed it yet, so I'm reverting my ACK and please add the tests and make sure that RNG schema fits.
Thanks, Martin
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
participants (2)
-
Erik Skultety -
Martin Kletzander