[libvirt] [PATCH] node_device: Check return value for udev_new()

[Daniel P. Berrange]

On Wed, Nov 30, 2016 at 04:25:58PM +0100, Martin Kletzander 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 at linux.vnet.ibm.com>
> > Reviewed-by: Bjoern Walk <bwalk at linux.vnet.ibm.com>
> > Reviewed-by: Boris Fiuczynski <fiuczy at 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.html#udev-new
> > -     *
> > -     * 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?  Does it really
> mean just an OOM error?  Couldn't we add a proper error message?

RHEL-6 vintage udev can return NULL here too, and it does seem to be
just OOM related - other problems are ignored.

Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://entangle-photo.org       -o-    http://search.cpan.org/~danberr/ :|