[libvirt] [PATCH] Improve invalid argument checks for the public API

[Matthias Bolte]

2011/5/17 Eric Blake <eblake at 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