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

[Erik Skultety]

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 at redhat.com>
> > ---
> >  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.

>
> IMHO, we should only mandate a non-NULL 'ids' parameter when
> maxids > 0 a few lines earlier
>
> All the other legacy APIs share the same validation flaw so will
> also need fixing.

Right, I was too hasty, since gendispatch took care of the server side
automatically, I forgot to adjust the public APIs manually, will do :).

Erik