[{"id":30613,"web_url":"https://patchwork.libcamera.org/comment/30613/","msgid":"<whcxjqr6knh7ogoe7zsrtntnc2crcz7c4ig43i2ky7etrzqxbr@hiljht3t5b3s>","date":"2024-08-06T07:19:37","subject":"Re: [PATCH] libcamera: udev: Catch udev notification errors","submitter":{"id":184,"url":"https://patchwork.libcamera.org/api/people/184/","name":"Stefan Klug","email":"stefan.klug@ideasonboard.com"},"content":"Hi Kieran,\n\nThank you for the patch. \n\nOn Mon, Aug 05, 2024 at 11:31:48PM +0100, Kieran Bingham wrote:\n> The udev_monitor_receive_device() can return NULL on an error as\n> detailed in the man pages for the function.\n> \n> The udevNotify() handler in the DeviceEnumeratorUdev directly uses the\n> return value of udev_monitor_receive_device() in successive calls to\n> process the event without having first checked the udev_device.\n> \n> Ensure we identify, and handle events where the udev_device can not be\n> returned successfully.\n> \n> Bug: https://bugs.libcamera.org/show_bug.cgi?id=230\n> Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> ---\n> This is a proposed fix for the reported CI failure caught by Systemd\n> runners. It's not clear if this failure was an injected failure, startup\n> race or otherwise - but given the documentation of this function states\n> it can return null ... it seems valid to null check before passing into\n> other APIs especially when a trace is reported directly at this point.\n> \n> I don't presently have a way to replicate or reproduce the error either,\n> but this patch itself has passed our ci tests (which isn't too\n> unexpected as the change is quite minimal otherwise).\n> \n> \n>  src/libcamera/device_enumerator_udev.cpp | 6 ++++++\n>  1 file changed, 6 insertions(+)\n> \n> diff --git a/src/libcamera/device_enumerator_udev.cpp b/src/libcamera/device_enumerator_udev.cpp\n> index 01c70b6daa82..c6c78651baa0 100644\n> --- a/src/libcamera/device_enumerator_udev.cpp\n> +++ b/src/libcamera/device_enumerator_udev.cpp\n> @@ -332,6 +332,12 @@ int DeviceEnumeratorUdev::addV4L2Device(dev_t devnum)\n>  void DeviceEnumeratorUdev::udevNotify()\n>  {\n>  \tstruct udev_device *dev = udev_monitor_receive_device(monitor_);\n> +\tif (!dev) {\n> +\t\tLOG(DeviceEnumerator, Warning)\n> +\t\t\t<< \"Ignoring notfication received without a device\";\n\nIs that even worth a warning or only debug?\n\nAnyways:\nSigned-off-by: Stefan Klug <stefan.klug@ideasonboard.com> \n\nCheers,\nStefan\n\n> +\t\treturn;\n> +\t}\n> +\n>  \tstd::string_view action(udev_device_get_action(dev));\n>  \tstd::string_view deviceNode(udev_device_get_devnode(dev));\n>  \n> -- \n> 2.45.2\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 73BD6BE173\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue,  6 Aug 2024 07:19:43 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6138A63394;\n\tTue,  6 Aug 2024 09:19:42 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id AE9DC61955\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue,  6 Aug 2024 09:19:40 +0200 (CEST)","from ideasonboard.com (unknown\n\t[IPv6:2a00:6020:448c:6c00:7a68:2b6f:8265:aa72])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id A0FE12C5;\n\tTue,  6 Aug 2024 09:18:48 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"k70RL3GR\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1722928728;\n\tbh=4oTEJr8o5uM3G73Kz9uYgGup58VUynq5xY/vpq/TFog=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=k70RL3GR2oszwBwEpyvf2N7Kz3DVoHBsikhoSoHerjX9oV1fExNEsZEkkFeYXgUhZ\n\tP6hqXSmRFvbEkwsv9nuG0K0+TNrJyJ2NkZQcW2To/X5q8gNLN/viQo7LIHt4ZJ0Yqt\n\tAZq8YyDAk8QtpozrXQr799C6+iD4n4hXaIefp81U=","Date":"Tue, 6 Aug 2024 09:19:37 +0200","From":"Stefan Klug <stefan.klug@ideasonboard.com>","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"libcamera devel <libcamera-devel@lists.libcamera.org>","Subject":"Re: [PATCH] libcamera: udev: Catch udev notification errors","Message-ID":"<whcxjqr6knh7ogoe7zsrtntnc2crcz7c4ig43i2ky7etrzqxbr@hiljht3t5b3s>","References":"<20240805223148.30605-1-kieran.bingham@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20240805223148.30605-1-kieran.bingham@ideasonboard.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":30614,"web_url":"https://patchwork.libcamera.org/comment/30614/","msgid":"<20240806074053.GB32045@pendragon.ideasonboard.com>","date":"2024-08-06T07:40:53","subject":"Re: [PATCH] libcamera: udev: Catch udev notification errors","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Tue, Aug 06, 2024 at 09:19:37AM +0200, Stefan Klug wrote:\n> Hi Kieran,\n> \n> Thank you for the patch. \n> \n> On Mon, Aug 05, 2024 at 11:31:48PM +0100, Kieran Bingham wrote:\n> > The udev_monitor_receive_device() can return NULL on an error as\n> > detailed in the man pages for the function.\n> > \n> > The udevNotify() handler in the DeviceEnumeratorUdev directly uses the\n> > return value of udev_monitor_receive_device() in successive calls to\n> > process the event without having first checked the udev_device.\n> > \n> > Ensure we identify, and handle events where the udev_device can not be\n> > returned successfully.\n> > \n> > Bug: https://bugs.libcamera.org/show_bug.cgi?id=230\n> > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> > ---\n> > This is a proposed fix for the reported CI failure caught by Systemd\n> > runners. It's not clear if this failure was an injected failure, startup\n> > race or otherwise - but given the documentation of this function states\n> > it can return null ... it seems valid to null check before passing into\n> > other APIs especially when a trace is reported directly at this point.\n> > \n> > I don't presently have a way to replicate or reproduce the error either,\n> > but this patch itself has passed our ci tests (which isn't too\n> > unexpected as the change is quite minimal otherwise).\n> > \n> > \n> >  src/libcamera/device_enumerator_udev.cpp | 6 ++++++\n> >  1 file changed, 6 insertions(+)\n> > \n> > diff --git a/src/libcamera/device_enumerator_udev.cpp b/src/libcamera/device_enumerator_udev.cpp\n> > index 01c70b6daa82..c6c78651baa0 100644\n> > --- a/src/libcamera/device_enumerator_udev.cpp\n> > +++ b/src/libcamera/device_enumerator_udev.cpp\n> > @@ -332,6 +332,12 @@ int DeviceEnumeratorUdev::addV4L2Device(dev_t devnum)\n> >  void DeviceEnumeratorUdev::udevNotify()\n> >  {\n> >  \tstruct udev_device *dev = udev_monitor_receive_device(monitor_);\n> > +\tif (!dev) {\n> > +\t\tLOG(DeviceEnumerator, Warning)\n> > +\t\t\t<< \"Ignoring notfication received without a device\";\n> \n> Is that even worth a warning or only debug?\n\nAs we've never seen this product for years, I wonder why it's happening.\nA warning would help noticing the issue. On the other hand, are such\nerrors means to happen in rare ubt valid cases ? And then, are they\nmeant to be ignored, or do they indicate something is fundamentally\nwrong and action needs to be taken ? If no action is needed and there\nare no consequences, then a debug message is more than enough.\n\nIf we want to diagnose this, we need more information. The message\nshould print the value of errno, and it would be nice if the original\nreporter of the issues could then report the error number.\n\n> Anyways:\n> Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com> \n> \n> > +\t\treturn;\n> > +\t}\n> > +\n> >  \tstd::string_view action(udev_device_get_action(dev));\n> >  \tstd::string_view deviceNode(udev_device_get_devnode(dev));","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id C4EFAC323E\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue,  6 Aug 2024 07:41:19 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E63F36337E;\n\tTue,  6 Aug 2024 09:41:18 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C133661955\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue,  6 Aug 2024 09:41:16 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 73C012C5;\n\tTue,  6 Aug 2024 09:40:24 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"T/BgzgOj\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1722930024;\n\tbh=dsrG5w2vmjyJefRIHveY1akKnKFLi/QtPPlHM8zQXvI=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=T/BgzgOjRmHjcy/PuvZHbPkx7+XRzASAp4ve+P6UXW4DLwCUy8TZdfxWf6wFLv7RH\n\t60IpOzE8Rg6ca3qX6KO+/jbXkCt7iMBzJJsFbNuAMSF63g7dp24UV62D50yBqmrZ9Y\n\tH4gL6RbAHr01BpWIAAsLV0268c1rR42SWkMicA0k=","Date":"Tue, 6 Aug 2024 10:40:53 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Stefan Klug <stefan.klug@ideasonboard.com>","Cc":"Kieran Bingham <kieran.bingham@ideasonboard.com>,\n\tlibcamera devel <libcamera-devel@lists.libcamera.org>","Subject":"Re: [PATCH] libcamera: udev: Catch udev notification errors","Message-ID":"<20240806074053.GB32045@pendragon.ideasonboard.com>","References":"<20240805223148.30605-1-kieran.bingham@ideasonboard.com>\n\t<whcxjqr6knh7ogoe7zsrtntnc2crcz7c4ig43i2ky7etrzqxbr@hiljht3t5b3s>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<whcxjqr6knh7ogoe7zsrtntnc2crcz7c4ig43i2ky7etrzqxbr@hiljht3t5b3s>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":30617,"web_url":"https://patchwork.libcamera.org/comment/30617/","msgid":"<172293381295.1687952.13374539360947873166@ping.linuxembedded.co.uk>","date":"2024-08-06T08:43:32","subject":"Re: [PATCH] libcamera: udev: Catch udev notification errors","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Laurent Pinchart (2024-08-06 08:40:53)\n> On Tue, Aug 06, 2024 at 09:19:37AM +0200, Stefan Klug wrote:\n> > Hi Kieran,\n> > \n> > Thank you for the patch. \n> > \n> > On Mon, Aug 05, 2024 at 11:31:48PM +0100, Kieran Bingham wrote:\n> > > The udev_monitor_receive_device() can return NULL on an error as\n> > > detailed in the man pages for the function.\n> > > \n> > > The udevNotify() handler in the DeviceEnumeratorUdev directly uses the\n> > > return value of udev_monitor_receive_device() in successive calls to\n> > > process the event without having first checked the udev_device.\n> > > \n> > > Ensure we identify, and handle events where the udev_device can not be\n> > > returned successfully.\n> > > \n> > > Bug: https://bugs.libcamera.org/show_bug.cgi?id=230\n> > > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> > > ---\n> > > This is a proposed fix for the reported CI failure caught by Systemd\n> > > runners. It's not clear if this failure was an injected failure, startup\n> > > race or otherwise - but given the documentation of this function states\n> > > it can return null ... it seems valid to null check before passing into\n> > > other APIs especially when a trace is reported directly at this point.\n> > > \n> > > I don't presently have a way to replicate or reproduce the error either,\n> > > but this patch itself has passed our ci tests (which isn't too\n> > > unexpected as the change is quite minimal otherwise).\n> > > \n> > > \n> > >  src/libcamera/device_enumerator_udev.cpp | 6 ++++++\n> > >  1 file changed, 6 insertions(+)\n> > > \n> > > diff --git a/src/libcamera/device_enumerator_udev.cpp b/src/libcamera/device_enumerator_udev.cpp\n> > > index 01c70b6daa82..c6c78651baa0 100644\n> > > --- a/src/libcamera/device_enumerator_udev.cpp\n> > > +++ b/src/libcamera/device_enumerator_udev.cpp\n> > > @@ -332,6 +332,12 @@ int DeviceEnumeratorUdev::addV4L2Device(dev_t devnum)\n> > >  void DeviceEnumeratorUdev::udevNotify()\n> > >  {\n> > >     struct udev_device *dev = udev_monitor_receive_device(monitor_);\n> > > +   if (!dev) {\n> > > +           LOG(DeviceEnumerator, Warning)\n> > > +                   << \"Ignoring notfication received without a device\";\n> > \n> > Is that even worth a warning or only debug?\n> \n> As we've never seen this product for years, I wonder why it's happening.\n> A warning would help noticing the issue. On the other hand, are such\n> errors means to happen in rare ubt valid cases ? And then, are they\n> meant to be ignored, or do they indicate something is fundamentally\n> wrong and action needs to be taken ? If no action is needed and there\n> are no consequences, then a debug message is more than enough.\n> \n> If we want to diagnose this, we need more information. The message\n> should print the value of errno, and it would be nice if the original\n> reporter of the issues could then report the error number.\n\nOh interesting idea. I didn't notice if the man pages reference setting\nerrno - but it's worth reporting to help diagnose the issue for sure.\n\n https://github.com/systemd/systemd/blob/5a5a7093b8e054cf3bfa65716ffe77a5d93647ff/src/libudev/libudev-monitor.c#L239\ncalls return_with_errno(NULL, r);, so I think errno is indeed\nworthwhile reporting.\n\nI'll update in v2.\n\n--\nKieran\n\n\n> > Anyways:\n> > Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com> \n> > \n> > > +           return;\n> > > +   }\n> > > +\n> > >     std::string_view action(udev_device_get_action(dev));\n> > >     std::string_view deviceNode(udev_device_get_devnode(dev));\n> \n> -- \n> Regards,\n> \n> Laurent Pinchart","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 7BA78C323E\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue,  6 Aug 2024 08:43:39 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A92416337E;\n\tTue,  6 Aug 2024 10:43:38 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 42CA461955\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue,  6 Aug 2024 10:43:36 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 216FB4CD;\n\tTue,  6 Aug 2024 10:42:44 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"i8sUftbe\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1722933764;\n\tbh=3DtiH56qSvtyhB8RlCQijhosdl/JVKjCFc76xNuuKXY=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=i8sUftbeA7RlNZfzmqM1ZvNWY2OjtE82CmxYLDlioboB6zrlEJzqS58NK47WAWi8g\n\tnl4tkvfSHbWLNPR3fQZECHhGiGgiHQGtMe9qMVA3uKYUxSi1Eyo3kMf24UsMpBK73M\n\tQBURdI37LCL/OvNptqzpmofMjVZdVWs0rvJOMQ3E=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20240806074053.GB32045@pendragon.ideasonboard.com>","References":"<20240805223148.30605-1-kieran.bingham@ideasonboard.com>\n\t<whcxjqr6knh7ogoe7zsrtntnc2crcz7c4ig43i2ky7etrzqxbr@hiljht3t5b3s>\n\t<20240806074053.GB32045@pendragon.ideasonboard.com>","Subject":"Re: [PATCH] libcamera: udev: Catch udev notification errors","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"libcamera devel <libcamera-devel@lists.libcamera.org>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>,\n\tStefan Klug <stefan.klug@ideasonboard.com>","Date":"Tue, 06 Aug 2024 09:43:32 +0100","Message-ID":"<172293381295.1687952.13374539360947873166@ping.linuxembedded.co.uk>","User-Agent":"alot/0.10","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":30683,"web_url":"https://patchwork.libcamera.org/comment/30683/","msgid":"<172303857327.1687952.13715747747848525592@ping.linuxembedded.co.uk>","date":"2024-08-07T13:49:33","subject":"Re: [PATCH] libcamera: udev: Catch udev notification errors","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Stefan Klug (2024-08-06 08:19:37)\n> Hi Kieran,\n> \n> Thank you for the patch. \n> \n> On Mon, Aug 05, 2024 at 11:31:48PM +0100, Kieran Bingham wrote:\n> > The udev_monitor_receive_device() can return NULL on an error as\n> > detailed in the man pages for the function.\n> > \n> > The udevNotify() handler in the DeviceEnumeratorUdev directly uses the\n> > return value of udev_monitor_receive_device() in successive calls to\n> > process the event without having first checked the udev_device.\n> > \n> > Ensure we identify, and handle events where the udev_device can not be\n> > returned successfully.\n> > \n> > Bug: https://bugs.libcamera.org/show_bug.cgi?id=230\n> > Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> > ---\n> > This is a proposed fix for the reported CI failure caught by Systemd\n> > runners. It's not clear if this failure was an injected failure, startup\n> > race or otherwise - but given the documentation of this function states\n> > it can return null ... it seems valid to null check before passing into\n> > other APIs especially when a trace is reported directly at this point.\n> > \n> > I don't presently have a way to replicate or reproduce the error either,\n> > but this patch itself has passed our ci tests (which isn't too\n> > unexpected as the change is quite minimal otherwise).\n> > \n> > \n> >  src/libcamera/device_enumerator_udev.cpp | 6 ++++++\n> >  1 file changed, 6 insertions(+)\n> > \n> > diff --git a/src/libcamera/device_enumerator_udev.cpp b/src/libcamera/device_enumerator_udev.cpp\n> > index 01c70b6daa82..c6c78651baa0 100644\n> > --- a/src/libcamera/device_enumerator_udev.cpp\n> > +++ b/src/libcamera/device_enumerator_udev.cpp\n> > @@ -332,6 +332,12 @@ int DeviceEnumeratorUdev::addV4L2Device(dev_t devnum)\n> >  void DeviceEnumeratorUdev::udevNotify()\n> >  {\n> >       struct udev_device *dev = udev_monitor_receive_device(monitor_);\n> > +     if (!dev) {\n> > +             LOG(DeviceEnumerator, Warning)\n> > +                     << \"Ignoring notfication received without a device\";\n> \n> Is that even worth a warning or only debug?\n> \n> Anyways:\n> Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com> \n\nThanks, but I'll assume that was \"Reviewed-by: \" :-)\n\n> \n> Cheers,\n> Stefan\n> \n> > +             return;\n> > +     }\n> > +\n> >       std::string_view action(udev_device_get_action(dev));\n> >       std::string_view deviceNode(udev_device_get_devnode(dev));\n> >  \n> > -- \n> > 2.45.2\n> >","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 73F2EBE173\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed,  7 Aug 2024 13:49:38 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B82586337F;\n\tWed,  7 Aug 2024 15:49:37 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 944456337E\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed,  7 Aug 2024 15:49:35 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id A6BEB6AF;\n\tWed,  7 Aug 2024 15:48:42 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"ajYdRN3r\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1723038522;\n\tbh=r/Oq6LsQZylzekKOUTUh7svWOV+W9eMaenGt+XoxX2s=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=ajYdRN3rEwWqqMmeZQETXDJXj3xDUk/tNrn/p1pYMSMl9TaGydkxOfUjzIHc7oTMt\n\tl0hgDJnVYr/zR/aGI6KtoewpZVokcrBxsJhiqePtM0ZBJMoafYUb53ltOKcasu1AW7\n\t1FSGpjboKEg7K4RXJZTs1XVt3/04qq0N9ltX/m08=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<whcxjqr6knh7ogoe7zsrtntnc2crcz7c4ig43i2ky7etrzqxbr@hiljht3t5b3s>","References":"<20240805223148.30605-1-kieran.bingham@ideasonboard.com>\n\t<whcxjqr6knh7ogoe7zsrtntnc2crcz7c4ig43i2ky7etrzqxbr@hiljht3t5b3s>","Subject":"Re: [PATCH] libcamera: udev: Catch udev notification errors","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"libcamera devel <libcamera-devel@lists.libcamera.org>","To":"Stefan Klug <stefan.klug@ideasonboard.com>","Date":"Wed, 07 Aug 2024 14:49:33 +0100","Message-ID":"<172303857327.1687952.13715747747848525592@ping.linuxembedded.co.uk>","User-Agent":"alot/0.10","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]