Re: [libvirt] [PATCH 3/3] libvirt-domain: virConnectListDomains: Return 0 on zero-size buffer

On Mon, Nov 18, 2019 at 02:26:31PM +0100, Erik Skultety wrote: > On Mon, Nov 18, 2019 at 01:04:01PM +0000, Daniel P. Berrangé wrote: > > On Mon, Nov 18, 2019 at 01:18:19PM +0100, Erik Skultety wrote: > > > It doesn't make sense to pass a target buffer into an API, declaring its > > > size as 0 and expect some meaningful result. Since this used to work > > > pre-Glib era, we shouldn't end with an error, but we can return 0 > > > for the number of domains immediately, instead of calling into the > > > daemon, which is exactly what the daemon would have returned anyway. > > > > Passing in size as 0 is going to be normal practice, given the calling > > convention of this API design. > > > > > > > > Signed-off-by: Erik Skultety <eskultet(a)redhat.com&gt; > > > --- > > > src/libvirt-domain.c | 3 +++ > > > 1 file changed, 3 insertions(+) > > > > > > diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c > > > index 02622cb2ca..0def40fdf7 100644 > > > --- a/src/libvirt-domain.c > > > +++ b/src/libvirt-domain.c > > > @@ -62,6 +62,9 @@ virConnectListDomains(virConnectPtr conn, int *ids, int maxids) > > > virCheckNonNullArgGoto(ids, error); > > > virCheckNonNegativeArgGoto(maxids, error); > > > > > > + if (maxids == 0) > > > + return 0; > > > > This is too late really, as we alrady checked 'ids'. > > Why is ^this a problem? The pointer has to be allocated prior to calling into > the API, so a failure on a NULL pointer on client-side is fine. On server side, > the issue is remediated much earlier in the RPC dispatch code. The current regression is caused server-side, but I'm saying the problem in general is pre-existing even client side. Chances are that people have already had to work around this when calling it client side. If we fix this though, we should fix it so that we aovid the problem both client & server side. IOW, rather than taking the approach to avoiding calling the API when num==0, make it valid to do the call, so that our public API's behaviour isn't dependant on whether the client's malloc() returns NULL or not for a zero sized allocation.