2011/5/18 Matthias Bolte <matthias.bo...@googlemail.com>: > 2011/5/18 Daniel Veillard <veill...@redhat.com>: >> On Tue, May 17, 2011 at 09:46:34AM -0600, Eric Blake wrote: >>> 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. >>> >>> > >>> >> 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. >>> >>> This affects Hu's patch series, where we are adding >>> virDomainSetSchedulerParametersFlags, which should have the same semantics. >> >> I was somehow remembering that we had let open the possibility to not >> provide the full array. Looking at the function comment which is the API >> contract there was nothing about it, so I looked at qemu backend and >> it fails if nparam != 1 , so at least that driver implemented the strict >> API. >> We need to decide either way, possibly by looking first at all API >> entry points using that mechanism array pointer + array size, and see >> if we are likely to break any app if we make it strict. Or we could >> decide to allow 0 and NULL and in that case we need to document >> explicitely the semantic. >> However for virDomainGetSchedulerParameters() it's relatively clear >> (well that could possibly be improved a bit) that the application must >> call virDomainGetSchedulerType() first to get nparams value, then >> allocate accordingly), so I would side on imposing the strict behaviour >> but make it a bit clearer. >> >> Daniel > > I'm working on a series to clean this up and go for the stricter > semantic of the Get*Parameters functions. > > Matthias >
The initial patch has been superseded by this series: https://www.redhat.com/archives/libvir-list/2011-May/msg01194.html Matthias -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list