Re: [PATCH] Improve invalid argument checks for the public API

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



2011/5/17 Eric Blake <eblake@xxxxxxxxxx>:
> 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

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]