2:58 a.m.
This happens because of the switch to glib whose method g_malloc0 actually
correctly returns NULL on size 0 which doesn't make sense to do. The outcome of
that is that because virAllocN always returns 0, the dispatch code never fails
allocation and passes the NULL pointer to the server-side public API which
actually checks that and fails.
https://bugzilla.redhat.com/show_bug.cgi?id=1772842
Since v1:
- reworked the original patch 3/3 (now 4/4) by introducing a
virCheckNonNull-style macro which checks whether the caller-provided array
isn't by any chance NULL while the declared size of this array is a
positive int (Dan's suggestion)
-> apart from the single scenario check (NULL && n > 0), the other
combinations are allowed for the legacy APIs letting the server-side
handle this
- added patch 3/4 when I realized one of the legacy APIs which didn't have
the dispatch code autogenerated needed adjustment too
- I deliberately didn't touch the APIs using typed params, as half of them
are setters and the other half is supposed to be called twice, so we
already have various NULL checks in place
- there were a few legacy APIs that already had some protection in the
remote dispatch code, so I didn't touch the client side for those, IMO we
can always treat those on case-by-case basis if a similar issue arises
than risk breaking one third of libvirt now :P
Erik Skultety (4):
rpc: gendispatch: Fix a couple of places adding trailing spaces
rpc: gendispatch: Add a check for zero size client-side buffers
remote: Add a check for zero sized client-side buffers
libvirt-<module>: Check caller-provided buffers to be NULL with size >
0
src/internal.h | 13 +++++++++++++
src/libvirt-domain-snapshot.c | 4 ++--
src/libvirt-domain.c | 21 ++++++---------------
src/libvirt-host.c | 2 +-
src/libvirt-interface.c | 4 ++--
src/libvirt-network.c | 4 ++--
src/libvirt-nodedev.c | 4 ++--
src/libvirt-nwfilter.c | 2 +-
src/libvirt-secret.c | 2 +-
src/libvirt-storage.c | 6 +++---
src/remote/remote_daemon_dispatch.c | 6 ++++++
src/rpc/gendispatch.pl | 12 ++++++++++--
12 files changed, 49 insertions(+), 31 deletions(-)
--
2.23.0
2:58 a.m.
New subject: [libvirt] [PATCH v2 1/4] rpc: gendispatch: Fix a couple of places adding trailing spaces
Signed-off-by: Erik Skultety <eskultet(a)redhat.com>
---
src/rpc/gendispatch.pl | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl
index 7c868191d1..8656c8f205 100755
--- a/src/rpc/gendispatch.pl
+++ b/src/rpc/gendispatch.pl
@@ -1062,7 +1062,7 @@ elsif ($mode eq "server") {
print "\n";
print " $conn_type $conn_var = $conn_method(client);\n";
- print " if (!$conn_var) \n";
+ print " if (!$conn_var)\n";
print " goto cleanup;\n";
print "\n";
@@ -1144,7 +1144,7 @@ elsif ($mode eq "server") {
print "\n";
} else {
if ($modern_ret_as_list) {
- print " if ((nresults = \n";
+ print " if ((nresults =\n";
my $indln = " $prefix$call->{ProcName}(";
print $indln;
print join(",\n" . ' ' x (length $indln), @args_list);
--
2.23.0
4:45 a.m.
New subject: [libvirt] [PATCH v2 1/4] rpc: gendispatch: Fix a couple of places adding trailing spaces
On Thu, Nov 21, 2019 at 09:58:29AM +0100, Erik Skultety wrote:
Signed-off-by: Erik Skultety <eskultet(a)redhat.com>
---
src/rpc/gendispatch.pl | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
Reviewed-by: Daniel P. Berrangé <berrange(a)redhat.com>
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
2:58 a.m.
New subject: [libvirt] [PATCH v2 2/4] rpc: gendispatch: Add a check for zero size client-side buffers
After libvirt switched to GLib, we also started to use glib allocation
primitives as of commit e85e34f3. Unlike malloc which is ambiguous with
regards to size == 0 (which in our case returned a unique pointer safe
to be passed to free), g_malloc0 strictly returns NULL on size == 0.
This change broke our legacy APIs which rely on the caller to
pre-allocate the target buffer to hold the results and pass the buffer
size as an argument. Since it makes very little sense to call an API
with nowhere to store the results, fix this by returning 0 directly in
such case in the RPC dispatch code - there are modern API equivalents
allocating the target buffer automatically anyway.
https://bugzilla.redhat.com/show_bug.cgi?id=1772842
Signed-off-by: Erik Skultety <eskultet(a)redhat.com>
---
src/rpc/gendispatch.pl | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl
index 8656c8f205..524d31f741 100755
--- a/src/rpc/gendispatch.pl
+++ b/src/rpc/gendispatch.pl
@@ -1073,6 +1073,14 @@ elsif ($mode eq "server") {
print " goto cleanup;\n";
print " }\n";
print "\n";
+
+
+ print " if (args->$single_ret_list_max_var == 0) {\n";
+ print " ret->$single_ret_list_name.${single_ret_list_name}_len
= 0;\n";
+ print " rv = 0;\n";
+ print " goto cleanup;\n";
+ print " }\n";
+ print "\n";
}
print join("\n", @getters_list);
--
2.23.0
4:48 a.m.
New subject: [libvirt] [PATCH v2 2/4] rpc: gendispatch: Add a check for zero size client-side buffers
On Thu, Nov 21, 2019 at 09:58:30AM +0100, Erik Skultety wrote:
After libvirt switched to GLib, we also started to use glib
allocation
primitives as of commit e85e34f3. Unlike malloc which is ambiguous with
regards to size == 0 (which in our case returned a unique pointer safe
to be passed to free), g_malloc0 strictly returns NULL on size == 0.
This change broke our legacy APIs which rely on the caller to
pre-allocate the target buffer to hold the results and pass the buffer
size as an argument. Since it makes very little sense to call an API
with nowhere to store the results, fix this by returning 0 directly in
such case in the RPC dispatch code - there are modern API equivalents
allocating the target buffer automatically anyway.
As mentioned before, I don't think we should be short-circuiting the
driver APIs like this. It is still valid to call the APIs with zero
length list such that we get access control checks validated.
https://bugzilla.redhat.com/show_bug.cgi?id=1772842
Signed-off-by: Erik Skultety <eskultet(a)redhat.com>
---
src/rpc/gendispatch.pl | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/src/rpc/gendispatch.pl b/src/rpc/gendispatch.pl
index 8656c8f205..524d31f741 100755
--- a/src/rpc/gendispatch.pl
+++ b/src/rpc/gendispatch.pl
@@ -1073,6 +1073,14 @@ elsif ($mode eq "server") {
print " goto cleanup;\n";
print " }\n";
print "\n";
+
+
+ print " if (args->$single_ret_list_max_var == 0) {\n";
+ print "
ret->$single_ret_list_name.${single_ret_list_name}_len = 0;\n";
+ print " rv = 0;\n";
+ print " goto cleanup;\n";
+ print " }\n";
+ print "\n";
}
print join("\n", @getters_list);
--
2.23.0
--
libvir-list mailing list
libvir-list(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
2:58 a.m.
New subject: [libvirt] [PATCH v2 3/4] remote: Add a check for zero sized client-side buffers
Adjustment to the dispatch code which is not generated by
gendispatch.pl.
Signed-off-by: Erik Skultety <eskultet(a)redhat.com>
---
src/remote/remote_daemon_dispatch.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/src/remote/remote_daemon_dispatch.c b/src/remote/remote_daemon_dispatch.c
index f369ffb02a..1d75ec3d37 100644
--- a/src/remote/remote_daemon_dispatch.c
+++ b/src/remote/remote_daemon_dispatch.c
@@ -2753,6 +2753,12 @@ remoteDispatchDomainGetVcpuPinInfo(virNetServerPtr server
G_GNUC_UNUSED,
goto cleanup;
}
+ if (args->ncpumaps == 0) {
+ ret->cpumaps.cpumaps_len = 0;
+ rv = 0;
+ goto cleanup;
+ }
+
if (INT_MULTIPLY_OVERFLOW(args->ncpumaps, args->maplen) ||
args->ncpumaps * args->maplen > REMOTE_CPUMAPS_MAX) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("maxinfo * maplen
> REMOTE_CPUMAPS_MAX"));
--
2.23.0
4:52 a.m.
New subject: [libvirt] [PATCH v2 3/4] remote: Add a check for zero sized client-side buffers
On Thu, Nov 21, 2019 at 09:58:31AM +0100, Erik Skultety wrote:
Adjustment to the dispatch code which is not generated by
gendispatch.pl.
Signed-off-by: Erik Skultety <eskultet(a)redhat.com>
---
src/remote/remote_daemon_dispatch.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/src/remote/remote_daemon_dispatch.c b/src/remote/remote_daemon_dispatch.c
index f369ffb02a..1d75ec3d37 100644
--- a/src/remote/remote_daemon_dispatch.c
+++ b/src/remote/remote_daemon_dispatch.c
@@ -2753,6 +2753,12 @@ remoteDispatchDomainGetVcpuPinInfo(virNetServerPtr server
G_GNUC_UNUSED,
goto cleanup;
}
+ if (args->ncpumaps == 0) {
+ ret->cpumaps.cpumaps_len = 0;
+ rv = 0;
+ goto cleanup;
+ }
Same comment as previous patch, I don't think we should be short-circuiting
the drivers in our dispatch code. Patch 4 makes this redundant by fixing
the root cause bug.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
2:58 a.m.
New subject: [libvirt] [PATCH v2 4/4] libvirt-<module>: Check caller-provided buffers to be NULL with size > 0
Pre-Glib era which used malloc allowed the size of the client-side
buffers to be declared as 0, because malloc documents that it can either
return 0 or a unique pointer on 0 size allocations.
With glib this doesn't work anymore, because glib documents that for
such allocation requests NULL is always returned which results in an
error in our public API checks server-side.
This patch complements the fix in the RPC layer by explicitly erroring
out on the following combination of args used by our legacy APIs (their
moder equivalents don't suffer from this):
function(caller-allocated-array, size, ...) {
if (!caller-allocated-array && size > 0)
return error;
}
treating everything else as a valid input and potentially let that fail
on the server-side rather than client-side.
Signed-off-by: Erik Skultety <eskultet(a)redhat.com>
---
src/internal.h | 13 +++++++++++++
src/libvirt-domain-snapshot.c | 4 ++--
src/libvirt-domain.c | 21 ++++++---------------
src/libvirt-host.c | 2 +-
src/libvirt-interface.c | 4 ++--
src/libvirt-network.c | 4 ++--
src/libvirt-nodedev.c | 4 ++--
src/libvirt-nwfilter.c | 2 +-
src/libvirt-secret.c | 2 +-
src/libvirt-storage.c | 6 +++---
10 files changed, 33 insertions(+), 29 deletions(-)
diff --git a/src/internal.h b/src/internal.h
index 0ff9f496ac..bcc5a1c157 100644
--- a/src/internal.h
+++ b/src/internal.h
@@ -429,6 +429,19 @@
} \
} while (0)
+/* This check is intended to be used with legacy APIs only which expect the
+ * caller to pre-allocate the target buffer.
+ * We want to allow callers pass NULL arrays if the size is declared as 0 and
+ * still succeed in calling the API.
+ */
+#define virCheckNonNullArrayArgGoto(argname, argsize, label) \
+ do { \
+ if (!argname && argsize > 0) { \
+ virReportInvalidNonNullArg(argname); \
+ goto label; \
+ } \
+ } while (0)
+
/* Count leading zeros in an unsigned int.
*
diff --git a/src/libvirt-domain-snapshot.c b/src/libvirt-domain-snapshot.c
index 20a3bc5545..33593e11e9 100644
--- a/src/libvirt-domain-snapshot.c
+++ b/src/libvirt-domain-snapshot.c
@@ -398,7 +398,7 @@ virDomainSnapshotListNames(virDomainPtr domain, char **names, int
nameslen,
virCheckDomainReturn(domain, -1);
conn = domain->conn;
- virCheckNonNullArgGoto(names, error);
+ virCheckNonNullArrayArgGoto(names, nameslen, error);
virCheckNonNegativeArgGoto(nameslen, error);
if (conn->driver->domainSnapshotListNames) {
@@ -600,7 +600,7 @@ virDomainSnapshotListChildrenNames(virDomainSnapshotPtr snapshot,
virCheckDomainSnapshotReturn(snapshot, -1);
conn = snapshot->domain->conn;
- virCheckNonNullArgGoto(names, error);
+ virCheckNonNullArrayArgGoto(names, nameslen, error);
virCheckNonNegativeArgGoto(nameslen, error);
if (conn->driver->domainSnapshotListChildrenNames) {
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 02622cb2ca..17cdd5bcaf 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -59,7 +59,7 @@ virConnectListDomains(virConnectPtr conn, int *ids, int maxids)
virResetLastError();
virCheckConnectReturn(conn, -1);
- virCheckNonNullArgGoto(ids, error);
+ virCheckNonNullArrayArgGoto(ids, maxids, error);
virCheckNonNegativeArgGoto(maxids, error);
if (conn->driver->connectListDomains) {
@@ -6386,7 +6386,7 @@ virConnectListDefinedDomains(virConnectPtr conn, char **const
names,
virResetLastError();
virCheckConnectReturn(conn, -1);
- virCheckNonNullArgGoto(names, error);
+ virCheckNonNullArrayArgGoto(names, maxnames, error);
virCheckNonNegativeArgGoto(maxnames, error);
if (conn->driver->connectListDefinedDomains) {
@@ -7298,7 +7298,7 @@ virDomainGetVcpuPinInfo(virDomainPtr domain, int ncpumaps,
virCheckDomainReturn(domain, -1);
conn = domain->conn;
- virCheckNonNullArgGoto(cpumaps, error);
+ virCheckNonNullArrayArgGoto(cpumaps, ncpumaps, error);
virCheckPositiveArgGoto(ncpumaps, error);
virCheckPositiveArgGoto(maplen, error);
@@ -10996,10 +10996,7 @@ virDomainGetDiskErrors(virDomainPtr dom,
virCheckDomainReturn(dom, -1);
- if (maxerrors)
- virCheckNonNullArgGoto(errors, error);
- else
- virCheckNullArgGoto(errors, error);
+ virCheckNonNullArrayArgGoto(errors, maxerrors, error);
if (dom->conn->driver->domainGetDiskErrors) {
int ret = dom->conn->driver->domainGetDiskErrors(dom, errors,
@@ -11136,10 +11133,7 @@ virDomainFSFreeze(virDomainPtr dom,
virCheckDomainReturn(dom, -1);
virCheckReadOnlyGoto(dom->conn->flags, error);
- if (nmountpoints)
- virCheckNonNullArgGoto(mountpoints, error);
- else
- virCheckNullArgGoto(mountpoints, error);
+ virCheckNonNullArrayArgGoto(mountpoints, nmountpoints, error);
if (dom->conn->driver->domainFSFreeze) {
int ret = dom->conn->driver->domainFSFreeze(
@@ -11181,10 +11175,7 @@ virDomainFSThaw(virDomainPtr dom,
virCheckDomainReturn(dom, -1);
virCheckReadOnlyGoto(dom->conn->flags, error);
- if (nmountpoints)
- virCheckNonNullArgGoto(mountpoints, error);
- else
- virCheckNullArgGoto(mountpoints, error);
+ virCheckNonNullArrayArgGoto(mountpoints, nmountpoints, error);
if (dom->conn->driver->domainFSThaw) {
int ret = dom->conn->driver->domainFSThaw(
diff --git a/src/libvirt-host.c b/src/libvirt-host.c
index 221a1b7a43..94ba5a8e80 100644
--- a/src/libvirt-host.c
+++ b/src/libvirt-host.c
@@ -910,7 +910,7 @@ virNodeGetCellsFreeMemory(virConnectPtr conn, unsigned long long
*freeMems,
virResetLastError();
virCheckConnectReturn(conn, -1);
- virCheckNonNullArgGoto(freeMems, error);
+ virCheckNonNullArrayArgGoto(freeMems, maxCells, error);
virCheckPositiveArgGoto(maxCells, error);
virCheckNonNegativeArgGoto(startCell, error);
diff --git a/src/libvirt-interface.c b/src/libvirt-interface.c
index 7228ddca57..2d2df68131 100644
--- a/src/libvirt-interface.c
+++ b/src/libvirt-interface.c
@@ -166,7 +166,7 @@ virConnectListInterfaces(virConnectPtr conn, char **const names, int
maxnames)
virResetLastError();
virCheckConnectReturn(conn, -1);
- virCheckNonNullArgGoto(names, error);
+ virCheckNonNullArrayArgGoto(names, maxnames, error);
virCheckNonNegativeArgGoto(maxnames, error);
if (conn->interfaceDriver &&
conn->interfaceDriver->connectListInterfaces) {
@@ -245,7 +245,7 @@ virConnectListDefinedInterfaces(virConnectPtr conn,
virResetLastError();
virCheckConnectReturn(conn, -1);
- virCheckNonNullArgGoto(names, error);
+ virCheckNonNullArrayArgGoto(names, maxnames, error);
virCheckNonNegativeArgGoto(maxnames, error);
if (conn->interfaceDriver &&
conn->interfaceDriver->connectListDefinedInterfaces) {
diff --git a/src/libvirt-network.c b/src/libvirt-network.c
index 146ccc5e4a..09e24fb0a8 100644
--- a/src/libvirt-network.c
+++ b/src/libvirt-network.c
@@ -175,7 +175,7 @@ virConnectListNetworks(virConnectPtr conn, char **const names, int
maxnames)
virResetLastError();
virCheckConnectReturn(conn, -1);
- virCheckNonNullArgGoto(names, error);
+ virCheckNonNullArrayArgGoto(names, maxnames, error);
virCheckNonNegativeArgGoto(maxnames, error);
if (conn->networkDriver && conn->networkDriver->connectListNetworks)
{
@@ -252,7 +252,7 @@ virConnectListDefinedNetworks(virConnectPtr conn, char **const names,
virResetLastError();
virCheckConnectReturn(conn, -1);
- virCheckNonNullArgGoto(names, error);
+ virCheckNonNullArrayArgGoto(names, maxnames, error);
virCheckNonNegativeArgGoto(maxnames, error);
if (conn->networkDriver &&
conn->networkDriver->connectListDefinedNetworks) {
diff --git a/src/libvirt-nodedev.c b/src/libvirt-nodedev.c
index 10050b193b..dce46b7181 100644
--- a/src/libvirt-nodedev.c
+++ b/src/libvirt-nodedev.c
@@ -169,7 +169,7 @@ virNodeListDevices(virConnectPtr conn,
virResetLastError();
virCheckConnectReturn(conn, -1);
- virCheckNonNullArgGoto(names, error);
+ virCheckNonNullArrayArgGoto(names, maxnames, error);
virCheckNonNegativeArgGoto(maxnames, error);
if (conn->nodeDeviceDriver &&
conn->nodeDeviceDriver->nodeListDevices) {
@@ -415,7 +415,7 @@ virNodeDeviceListCaps(virNodeDevicePtr dev,
virResetLastError();
virCheckNodeDeviceReturn(dev, -1);
- virCheckNonNullArgGoto(names, error);
+ virCheckNonNullArrayArgGoto(names, maxnames, error);
virCheckNonNegativeArgGoto(maxnames, error);
if (dev->conn->nodeDeviceDriver &&
dev->conn->nodeDeviceDriver->nodeDeviceListCaps) {
diff --git a/src/libvirt-nwfilter.c b/src/libvirt-nwfilter.c
index 16eceb6525..d28220db8a 100644
--- a/src/libvirt-nwfilter.c
+++ b/src/libvirt-nwfilter.c
@@ -127,7 +127,7 @@ virConnectListNWFilters(virConnectPtr conn, char **const names, int
maxnames)
virResetLastError();
virCheckConnectReturn(conn, -1);
- virCheckNonNullArgGoto(names, error);
+ virCheckNonNullArrayArgGoto(names, maxnames, error);
virCheckNonNegativeArgGoto(maxnames, error);
if (conn->nwfilterDriver &&
conn->nwfilterDriver->connectListNWFilters) {
diff --git a/src/libvirt-secret.c b/src/libvirt-secret.c
index 711c4fc580..33cbdd7b0b 100644
--- a/src/libvirt-secret.c
+++ b/src/libvirt-secret.c
@@ -166,7 +166,7 @@ virConnectListSecrets(virConnectPtr conn, char **uuids, int maxuuids)
virResetLastError();
virCheckConnectReturn(conn, -1);
- virCheckNonNullArgGoto(uuids, error);
+ virCheckNonNullArrayArgGoto(uuids, maxuuids, error);
virCheckNonNegativeArgGoto(maxuuids, error);
if (conn->secretDriver != NULL &&
conn->secretDriver->connectListSecrets != NULL) {
diff --git a/src/libvirt-storage.c b/src/libvirt-storage.c
index 05b2365692..0406fe84d3 100644
--- a/src/libvirt-storage.c
+++ b/src/libvirt-storage.c
@@ -197,7 +197,7 @@ virConnectListStoragePools(virConnectPtr conn,
virResetLastError();
virCheckConnectReturn(conn, -1);
- virCheckNonNullArgGoto(names, error);
+ virCheckNonNullArrayArgGoto(names, maxnames, error);
virCheckNonNegativeArgGoto(maxnames, error);
if (conn->storageDriver &&
conn->storageDriver->connectListStoragePools) {
@@ -277,7 +277,7 @@ virConnectListDefinedStoragePools(virConnectPtr conn,
virResetLastError();
virCheckConnectReturn(conn, -1);
- virCheckNonNullArgGoto(names, error);
+ virCheckNonNullArrayArgGoto(names, maxnames, error);
virCheckNonNegativeArgGoto(maxnames, error);
if (conn->storageDriver &&
conn->storageDriver->connectListDefinedStoragePools) {
@@ -1268,7 +1268,7 @@ virStoragePoolListVolumes(virStoragePoolPtr pool,
virResetLastError();
virCheckStoragePoolReturn(pool, -1);
- virCheckNonNullArgGoto(names, error);
+ virCheckNonNullArrayArgGoto(names, maxnames, error);
virCheckNonNegativeArgGoto(maxnames, error);
if (pool->conn->storageDriver &&
pool->conn->storageDriver->storagePoolListVolumes) {
--
2.23.0
4:50 a.m.
New subject: [libvirt] [PATCH v2 4/4] libvirt-<module>: Check caller-provided buffers to be NULL with size > 0
On Thu, Nov 21, 2019 at 09:58:32AM +0100, Erik Skultety wrote:
Pre-Glib era which used malloc allowed the size of the client-side
buffers to be declared as 0, because malloc documents that it can either
return 0 or a unique pointer on 0 size allocations.
With glib this doesn't work anymore, because glib documents that for
such allocation requests NULL is always returned which results in an
error in our public API checks server-side.
This patch complements the fix in the RPC layer by explicitly erroring
out on the following combination of args used by our legacy APIs (their
moder equivalents don't suffer from this):
function(caller-allocated-array, size, ...) {
if (!caller-allocated-array && size > 0)
return error;
}
treating everything else as a valid input and potentially let that fail
on the server-side rather than client-side.
Signed-off-by: Erik Skultety <eskultet(a)redhat.com>
---
src/internal.h | 13 +++++++++++++
src/libvirt-domain-snapshot.c | 4 ++--
src/libvirt-domain.c | 21 ++++++---------------
src/libvirt-host.c | 2 +-
src/libvirt-interface.c | 4 ++--
src/libvirt-network.c | 4 ++--
src/libvirt-nodedev.c | 4 ++--
src/libvirt-nwfilter.c | 2 +-
src/libvirt-secret.c | 2 +-
src/libvirt-storage.c | 6 +++---
10 files changed, 33 insertions(+), 29 deletions(-)
@@ -11136,10 +11133,7 @@ virDomainFSFreeze(virDomainPtr dom,
virCheckDomainReturn(dom, -1);
virCheckReadOnlyGoto(dom->conn->flags, error);
- if (nmountpoints)
- virCheckNonNullArgGoto(mountpoints, error);
- else
- virCheckNullArgGoto(mountpoints, error);
Interesting, so this actually returned an error if you did *not*
pass NULL when nmountpoints == 0, so this would be broken if
the caller's malloc() returned non-NULL region for size==0
+ virCheckNonNullArrayArgGoto(mountpoints, nmountpoints, error);
Reviewed-by: Daniel P. Berrangé <berrange(a)redhat.com>
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
1828
days inactive
1828
days old
8 comments
2 participants
participants (2)
-
Daniel P. Berrangé -
Erik Skultety