On 11/23/2010 11:49 AM, Eric Blake wrote:
> On 11/22/2010 02:35 PM, Cole Robinson wrote:
>>
>> Signed-off-by: Cole Robinson <crobinso@xxxxxxxxxx>
>> ---
>> src/conf/domain_conf.c | 11 +++++++++--
>> 1 files changed, 9 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
>> index 11a6280..045934d 100644
>> --- a/src/conf/domain_conf.c
>> +++ b/src/conf/domain_conf.c
>> @@ -4569,7 +4569,7 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps,
>> def->maxvcpus = 1;
>> } else {
>> def->maxvcpus = count;
>> - if (def->maxvcpus != count || count == 0) {
>> + if (count == 0) {
>
> At first glance, I was about to complain: Since def->maxvcpus is an
> unsigned short but count is an int, someone calling setvcpus 0x10001
> will silently overflow and end up setting def->maxvcpus == 1. In other
> words, you just deleted the 'def->maxvcpus != count' overflow check...
>
>> virDomainReportError(VIR_ERR_XML_ERROR,
>> _("invalid maxvcpus %lu"), count);
>> goto error;
>> @@ -4585,11 +4585,18 @@ static virDomainDefPtr virDomainDefParseXML(virCapsPtr caps,
>> def->vcpus = def->maxvcpus;
>> } else {
>> def->vcpus = count;
>> - if (def->vcpus != count || count == 0 || def->maxvcpus < count) {
>> + if (count == 0) {
>> virDomainReportError(VIR_ERR_XML_ERROR,
>> _("invalid current vcpus %lu"), count);
>> goto error;
>> }
>> +
>> + if (def->maxvcpus < count) {
>
> ...but this new code is an equally effective overflow check. No
> complaint after all; def is local, so it doesn't matter if we changed
> def->maxvcpus to an invalid value before detecting overflow. Thanks for
> cleaning this up for me.
>
Ahh, I didn't realize that check was for overflow, I thought it was
unintentional redundancy :/ Glad it worked out okay in the end!
Thanks,
Cole
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list