Re: [libvirt] [PATCH] Improve invalid argument checks for the public API
2011/5/17 Eric Blake <eblake(a)redhat.com>:
On 05/17/2011 07:14 AM, Daniel Veillard wrote:
> On Tue, May 17, 2011 at 02:45:44PM +0200, Matthias Bolte wrote:
>> ---
>> src/libvirt.c | 100 +++++++++++++++++++++++++++++++++++++++++++++++++++++---
>> 1 files changed, 94 insertions(+), 6 deletions(-)
>
>> @@ -5040,6 +5045,12 @@ virDomainGetSchedulerParameters(virDomainPtr domain,
>> virDispatchError(NULL);
>> return -1;
>> }
>> +
>> + if (params == NULL || nparams == NULL) {
>> + virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__);
>> + goto error;
>> + }
>
> I was just wondering here if params being NULL couldn't be used to
> find out the correct value for nparams, but looking at least at the
> qemu driver code we really expect the array there.
Let's fix that as a separate patch.
The actual semantic for (n)params is not completely defined, is it?
This just matches what (most of?) the driver currently do.
>
>> conn = domain->conn;
>>
>> if (conn->driver->domainGetSchedulerParameters) {
>> @@ -5084,6 +5095,12 @@ virDomainSetSchedulerParameters(virDomainPtr domain,
>> virDispatchError(NULL);
>> return -1;
>> }
>> +
>> + if (params == NULL) {
>> + virLibDomainError(VIR_ERR_INVALID_ARG, __FUNCTION__);
>> + goto error;
>> + }
Hmm, here we document that nparams can be <= the value returned by
virDomainGetSchedulerType; which means it can be 0, which means that
params can be NULL. I think we should change this to allow NULL,0 in
input as a way of querying the proper nparams size on output.
Why would you add a second way to query nparams?
This affects Hu's patch series, where we are adding
virDomainSetSchedulerParametersFlags, which should have the same semantics.
Well, it's documented that nparams can be less than what
virDomainGetSchedulerType returned, but that's not how is implemented
in the drivers. The drivers fail if you pass them nparams <
virDomainGetSchedulerType. Also it doesn't make that mush sense to
query less than all parameters as you cannot request for a specific
subset explicitly.
Matthias