Re: [libvirt] [PATCH] node_device: Check return value for udev_new()
On Wed, Nov 30, 2016 at 04:25 PM +0100, Martin Kletzander <mkletzan(a)redhat.com>
wrote:
On Wed, Nov 30, 2016 at 01:45:32PM +0100, Marc Hartmayer wrote:
>The comment was actually wrong as
>https://www.freedesktop.org/software/systemd/man/udev_new.html
>mentions that on failure NULL is returned.
>
>Signed-off-by: Marc Hartmayer <mhartmay(a)linux.vnet.ibm.com>
>Reviewed-by: Bjoern Walk <bwalk(a)linux.vnet.ibm.com>
>Reviewed-by: Boris Fiuczynski <fiuczy(a)linux.vnet.ibm.com>
>---
> src/node_device/node_device_udev.c | 10 ++++------
> 1 file changed, 4 insertions(+), 6 deletions(-)
>
>diff --git a/src/node_device/node_device_udev.c b/src/node_device/node_device_udev.c
>index 4b81312..4b0a875 100644
>--- a/src/node_device/node_device_udev.c
>+++ b/src/node_device/node_device_udev.c
>@@ -1491,13 +1491,11 @@ static int nodeStateInitialize(bool privileged,
> if (udevPCITranslateInit(privileged) < 0)
> goto cleanup;
>
>- /*
>- *
http://www.kernel.org/pub/linux/utils/kernel/hotplug/libudev/libudev-udev...
>- *
>- * indicates no return value other than success, so we don't check
>- * its return value.
>- */
> udev = udev_new();
>+ if (!udev) {
>+ virReportOOMError();
>+ goto cleanup;
>+ }
Is that true for other udevs and not just systemd-udev?
I'm not sure about other versions of udev but the NULL pointer is already
handled in udevStatInitialize() for udev_new() in a likewise manner.
Does it really mean just an OOM error?
It fails for systemd-udev if malloc/calloc fails => this is most
likely a OOM error at this point.
Couldn't we add a proper error message?
In udevStateInitialize() the error handling reports an internal error
but as the original error is caused by OOM I think we have to use
virReportOOMError().
--
Beste Grüße / Kind regards
Marc Hartmayer
IBM Deutschland Research & Development GmbH
Vorsitzende des Aufsichtsrats: Martina Koederitz
Geschäftsführung: Dirk Wittkopp
Sitz der Gesellschaft: Böblingen
Registergericht: Amtsgericht Stuttgart, HRB 243294