[PATCH] libcamera: udev: Catch udev notification errors

[Kieran Bingham]

Quoting Stefan Klug (2024-08-06 08:19:37)
> Hi Kieran,
> 
> Thank you for the patch. 
> 
> On Mon, Aug 05, 2024 at 11:31:48PM +0100, Kieran Bingham wrote:
> > The udev_monitor_receive_device() can return NULL on an error as
> > detailed in the man pages for the function.
> > 
> > The udevNotify() handler in the DeviceEnumeratorUdev directly uses the
> > return value of udev_monitor_receive_device() in successive calls to
> > process the event without having first checked the udev_device.
> > 
> > Ensure we identify, and handle events where the udev_device can not be
> > returned successfully.
> > 
> > Bug: https://bugs.libcamera.org/show_bug.cgi?id=230
> > Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> > ---
> > This is a proposed fix for the reported CI failure caught by Systemd
> > runners. It's not clear if this failure was an injected failure, startup
> > race or otherwise - but given the documentation of this function states
> > it can return null ... it seems valid to null check before passing into
> > other APIs especially when a trace is reported directly at this point.
> > 
> > I don't presently have a way to replicate or reproduce the error either,
> > but this patch itself has passed our ci tests (which isn't too
> > unexpected as the change is quite minimal otherwise).
> > 
> > 
> >  src/libcamera/device_enumerator_udev.cpp | 6 ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/src/libcamera/device_enumerator_udev.cpp b/src/libcamera/device_enumerator_udev.cpp
> > index 01c70b6daa82..c6c78651baa0 100644
> > --- a/src/libcamera/device_enumerator_udev.cpp
> > +++ b/src/libcamera/device_enumerator_udev.cpp
> > @@ -332,6 +332,12 @@ int DeviceEnumeratorUdev::addV4L2Device(dev_t devnum)
> >  void DeviceEnumeratorUdev::udevNotify()
> >  {
> >       struct udev_device *dev = udev_monitor_receive_device(monitor_);
> > +     if (!dev) {
> > +             LOG(DeviceEnumerator, Warning)
> > +                     << "Ignoring notfication received without a device";
> 
> Is that even worth a warning or only debug?
> 
> Anyways:
> Signed-off-by: Stefan Klug <stefan.klug at ideasonboard.com> 

Thanks, but I'll assume that was "Reviewed-by: " :-)

> 
> Cheers,
> Stefan
> 
> > +             return;
> > +     }
> > +
> >       std::string_view action(udev_device_get_action(dev));
> >       std::string_view deviceNode(udev_device_get_devnode(dev));
> >  
> > -- 
> > 2.45.2
> >