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@xxxxxxxxxx> > > --- > > 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 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
