Re: [libvirt] [PATCH] conf: Use UNIT_MAX for checking max value of unsigned int
On Monday, 14 November 2016 22:43:55 CET Nitesh Konkar wrote:
In the current scenario the controller parsing fails
when no index is specified .As the docs point out
that specifying index is optional, libvirt is expected
to fill the index without erroring out.
Signed-off-by: Nitesh Konkar <nitkon12(a)linux.vnet.ibm.com>
---
There's something I don't understand -- it looks to me there's a
mismatch between the test case you provide (and its output), and what
the code part for it actually does.
Before the Patch:
cat controller.xml
<controller model="virtio-scsi" type="scsi" />
Your XML has no index="" attribute.
virsh attach-device vm1 controller.xml --persistent
error: Failed to attach device from controller.xml
error: internal error: Cannot parse controller index -1
After the patch:
virsh attach-device vm1 controller.xml --persistent
Device attached successfully
---
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 6e008e2..c52e67f 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -8374,7 +8374,7 @@ virDomainControllerDefParseXML(xmlNodePtr node,
if (idx) {
Here "idx" is non-null only if the attribute is actually found, which
does not seem the case in the XML snippet above.
unsigned int idxVal;
if (virStrToLong_ui(idx, NULL, 10, &idxVal) < 0 ||
- idxVal > INT_MAX) {
+ idxVal > UINT_MAX) {
This looks wrong to me: UINT_MAX is the maximum value that is going to
fit into a 'unsigned int', so 'idxVal' will never be (even on error)
greater than that.
If the index must be non-negative, it looks like using virStrToLong_uip
instead of virStrToLong_ui could work fine in rejecting negative values
outright.
virReportError(VIR_ERR_INTERNAL_ERROR,
_("Cannot parse controller index %s"), idx);
Considering this, I think the XML you used in your testsmost probably
has index="-1" as attribute -- can you please confirm?
Thanks,
--
Pino Toscano
Attachments:
- signature.asc (application/pgp-signature — 819 bytes)