Re: [PATCH 6/8] tools/xenalyze: Fix off-by-one in MAX_CPUS range checks

[George Dunlap <george.dunlap@citrix.com>]

On 26/02/16 12:30, Ian Jackson wrote:
> George Dunlap writes ("[PATCH 6/8] tools/xenalyze: Fix off-by-one in MAX_CPUS range checks"):
>> Skip action / throw error if cpu/vcpu >= MAX_CPUS  rather than >.
>>
>> Also add an assertion to vcpu_find, to make future errors of this kind
>> not out-of-bounds.
> ...
>> +    /* "Graceful" handling of vid >= MAX_CPUS should be handled elsewhere */
>> +    if ( vid >= MAX_CPUS ) {
>> +        fprintf(stderr, "%s: vcpu %d exceeds MAX_CPUS %d!\n",
>> +                __func__, vid, MAX_CPUS);
>> +        error(ERR_ASSERT, NULL);
>> +    }
> 
> I'm not convinced by the existence of error(ERR_ASSERT,...).  What is
> wrong with assert() ?

Well one half of the reason for error() in general is to print out the
record which caused (or was involved in) the error before dying.  And
I'm guessing that once I decided I'd have error(ERR_ASSERT, xxx), that
for consistency I just decided to use error(ERR_ASSERT,...) everywhere.

But at least at this point, no instance of error(ERR_ASSERT...) actually
takes a pointer to a record, so that probably is something that could
just go away.

I'll send a new series with this updated.

 -George

> 
> If you agree that ERR_ASSERT should be got rid of, then you could
> start here...
> 
> But:
> 
> Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
> 


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel