[{"id":82,"web_url":"https://patchwork.libcamera.org/comment/82/","msgid":"<20181224114049.GF2364@archlinux>","date":"2018-12-24T11:40:49","subject":"Re: [libcamera-devel] [PATCH 06/12] libcamera: deviceenumerator:\n\tadd DeviceEnumeratorUdev class","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Niklas,\n\nOn Sun, Dec 23, 2018 at 12:00:35AM +0100, Niklas S??derlund wrote:\n> Provide a DeviceEnumeratorUdev class which is a specialization\n> of DeviceEnumerator which uses udev to enumerate information in the\n> system.\n>\n> Signed-off-by: Niklas Söderlund \n> ---\n> src/libcamera/deviceenumerator.cpp | 100 +++++++++++++++++++++++\n> src/libcamera/include/deviceenumerator.h | 15 ++++\n> 2 files changed, 115 insertions(+)\n>\n> diff --git a/src/libcamera/deviceenumerator.cpp b/src/libcamera/deviceenumerator.cpp\n> index b2f59017e94d14aa..f4c40bf0376ab453 100644\n> --- a/src/libcamera/deviceenumerator.cpp\n> +++ b/src/libcamera/deviceenumerator.cpp\n> @@ -6,6 +6,7 @@\n> */\n>\n> #include \n> +#include \n> #include \n> #include \n>\n> @@ -273,4 +274,103 @@ DeviceInfo *DeviceEnumerator::search(DeviceMatch &dm) const\n> \treturn info;\n> }\n>\n> +/* -----------------------------------------------------------------------------\n> + * Enumerator Udev\n> + */\n> +\n> +DeviceEnumeratorUdev::DeviceEnumeratorUdev()\n> +\t: udev_(NULL)\n> +{\n> +}\n\nSame question as other patches. Empty constructors with only memeber\ninitializer list might go in the header file?\n\n> +\n> +DeviceEnumeratorUdev::~DeviceEnumeratorUdev()\n> +{\n> +\tif (udev_)\n> +\t\tudev_unref(udev_);\n> +}\n> +\n> +int DeviceEnumeratorUdev::init()\n> +{\n> +\tif (udev_)\n> +\t\treturn -EBUSY;\n> +\n> +\tudev_ = udev_new();\n> +\tif (!udev_)\n> +\t\treturn -ENODEV;\n> +\n> +\treturn 0;\n> +}\n> +\n> +int DeviceEnumeratorUdev::enumerate()\n> +{\n> +\tstruct udev_enumerate *udev_enum = NULL;\n> +\tstruct udev_list_entry *ents, *ent;\n> +\tint ret;\n> +\n> +\tudev_enum = udev_enumerate_new(udev_);\n> +\tif (udev_enum == NULL)\n> +\t\treturn -ENOMEM;\n> +\n> +\tret = udev_enumerate_add_match_subsystem(udev_enum, \"media\");\n> +\tif (ret < 0)\n> +\t\tgoto done;\n> +\n> +\tret = udev_enumerate_scan_devices(udev_enum);\n> +\tif (ret < 0)\n> +\t\tgoto done;\n> +\n> +\tents = udev_enumerate_get_list_entry(udev_enum);\n> +\tif (ents == NULL)\n> +\t\tgoto done;\n> +\n> +\tudev_list_entry_foreach(ent, ents) {\n> +\t\tstruct udev_device *dev;\n> +\t\tconst char *devnode;\n> +\t\tconst char *syspath = udev_list_entry_get_name(ent);\n\nnit: you've sorted variables in reverse-xmas-tree order in this file,\ncould you re-sort here to keep them consistent?\n\n> +\n> +\t\tdev = udev_device_new_from_syspath(udev_, syspath);\n> +\t\tif (dev == NULL) {\n\nnullptr, or better !dev\n\n> +\t\t\tLOG(Error) << \"Failed to device for '\" << syspath << \"', skipping\";\n\nbreak to stay in 80 cols. We can span to 120 if it makes code more\nclear, but here it is not necessary\n\n\t\t\tLOG(Error) << \"Failed to device for '\"\n << syspath << \"', skipping\";\n\nAlso \"Failed to device\": seems like you've missed the verb :)\n\n> +\t\t\tcontinue;\n> +\t\t}\n> +\n> +\t\tdevnode = udev_device_get_devnode(dev);\n> +\t\tif (devnode == NULL) {\n\nnullptr, or better !dev\n\n> +\t\t\tudev_device_unref(dev);\n> +\t\t\tret = -ENODEV;\n> +\t\t\tgoto done;\n> +\t\t}\n> +\n> +\t\taddDevice(devnode);\n> +\n> +\t\tudev_device_unref(dev);\n> +\t}\n> +done:\n> +\tudev_enumerate_unref(udev_enum);\n> +\treturn ret >= 0 ? 0 : ret;\n> +}\n> +\n> +int DeviceEnumeratorUdev::lookupDevnode(std::string &devnode, int major, int minor)\n> +{\n> +\tstruct udev_device *device;\n> +\tconst char *name;\n> +\tdev_t devnum;\n\nThis might be discussed as well, and I pointed it out in review's of\nKieran's series as well. I probably like more declaring variables at the\nbeginning of the function, but it's common practice to declare them\nat initialization if possible.\n\nWhat do you think? Have a look at my series, where I tried to do that\nconsistently to make yourself an idea of how it would look like.\n\n> +\tint ret = 0;\n> +\n> +\tdevnum = makedev(major, minor);\n> +\tdevice = udev_device_new_from_devnum(udev_, 'c', devnum);\n> +\tif (!device)\n> +\t\treturn -ENODEV;\n> +\n> +\tname = udev_device_get_devnode(device);\n> +\tif (name)\n> +\t\tdevnode = name;\n> +\telse\n> +\t\tret = -ENODEV;\n> +\n> +\tudev_device_unref(device);\n> +\n> +\treturn ret;\n> +}\n> +\n> } /* namespace libcamera */\n> diff --git a/src/libcamera/include/deviceenumerator.h b/src/libcamera/include/deviceenumerator.h\n> index cbe17774edb7fcc5..2c7ff3f336ba127d 100644\n> --- a/src/libcamera/include/deviceenumerator.h\n> +++ b/src/libcamera/include/deviceenumerator.h\n> @@ -78,6 +78,21 @@ private:\n> \tvirtual int lookupDevnode(std::string &devnode, int major, int minor) = 0;\n> };\n>\n> +class DeviceEnumeratorUdev: public DeviceEnumerator\n> +{\n> +public:\n> +\tDeviceEnumeratorUdev();\n> +\t~DeviceEnumeratorUdev();\n> +\n> +\tint init() final;\n> +\tint enumerate() final;\n> +\n> +private:\n> +\tstruct udev *udev_;\n> +\n> +\tint lookupDevnode(std::string &devnode, int major, int minor) final;\n\nThis is what I was referring to in my comment to [00/12].\nHere it's easy to have udev enumerate entities, and in the same class\nuse udev to lookup for devnode paths.\n\nIf we go for the MediaDevice doing the entities enumeration, we need a\nway to provide a method for doing devnode lookup using different\nbackend.\n\n> +};\n\nThe use of final applied to all methods declared virtual in the base\nclass makes me wonder if the whole class shouldn't be declared final.\nBut at the same time I wonder if we're sure this will be final for\nreal :)\n\nIf not or the design thing, all minors comment :)\n\nThanks\n j\n\n> +\n> } /* namespace libcamera */\n>\n> #endif\t/* __LIBCAMERA_DEVICEENUMERATE_H__ */\n> --\n> 2.20.1\n>\n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"","Received":["from relay7-d.mail.gandi.net (relay7-d.mail.gandi.net\n\t[217.70.183.200])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A704060B1F\n\tfor ;\n\tMon, 24 Dec 2018 12:40:51 +0100 (CET)","from archlinux (host54-51-dynamic.16-87-r.retail.telecomitalia.it\n\t[87.16.51.54]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay7-d.mail.gandi.net (Postfix) with ESMTPSA id AE1832000C;\n\tMon, 24 Dec 2018 11:40:50 +0000 (UTC)"],"X-Originating-IP":"87.16.51.54","Date":"Mon, 24 Dec 2018 12:40:49 +0100","From":"Jacopo Mondi ","To":"Niklas S??derlund ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20181224114049.GF2364@archlinux>","References":"<20181222230041.29999-1-niklas.soderlund@ragnatech.se>\n\t<20181222230041.29999-7-niklas.soderlund@ragnatech.se>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"VuQYccsttdhdIfIP\"","Content-Disposition":"inline","In-Reply-To":"<20181222230041.29999-7-niklas.soderlund@ragnatech.se>","User-Agent":"Mutt/1.11.1 (2018-12-01)","Subject":"Re: [libcamera-devel] [PATCH 06/12] libcamera: deviceenumerator:\n\tadd DeviceEnumeratorUdev class","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Mon, 24 Dec 2018 11:40:51 -0000"}},{"id":89,"web_url":"https://patchwork.libcamera.org/comment/89/","msgid":"<20181228033151.GS19796@bigcity.dyn.berto.se>","date":"2018-12-28T03:31:51","subject":"Re: [libcamera-devel] [PATCH 06/12] libcamera: deviceenumerator:\n\tadd DeviceEnumeratorUdev class","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Jacopo,\n\nThanks for your feedback.\n\nOn 2018-12-24 12:40:49 +0100, Jacopo Mondi wrote:\n> Hi Niklas,\n> \n> On Sun, Dec 23, 2018 at 12:00:35AM +0100, Niklas S??derlund wrote:\n> > Provide a DeviceEnumeratorUdev class which is a specialization\n> > of DeviceEnumerator which uses udev to enumerate information in the\n> > system.\n> >\n> > Signed-off-by: Niklas Söderlund \n> > ---\n> > src/libcamera/deviceenumerator.cpp | 100 +++++++++++++++++++++++\n> > src/libcamera/include/deviceenumerator.h | 15 ++++\n> > 2 files changed, 115 insertions(+)\n> >\n> > diff --git a/src/libcamera/deviceenumerator.cpp b/src/libcamera/deviceenumerator.cpp\n> > index b2f59017e94d14aa..f4c40bf0376ab453 100644\n> > --- a/src/libcamera/deviceenumerator.cpp\n> > +++ b/src/libcamera/deviceenumerator.cpp\n> > @@ -6,6 +6,7 @@\n> > */\n> >\n> > #include \n> > +#include \n> > #include \n> > #include \n> >\n> > @@ -273,4 +274,103 @@ DeviceInfo *DeviceEnumerator::search(DeviceMatch &dm) const\n> > \treturn info;\n> > }\n> >\n> > +/* -----------------------------------------------------------------------------\n> > + * Enumerator Udev\n> > + */\n> > +\n> > +DeviceEnumeratorUdev::DeviceEnumeratorUdev()\n> > +\t: udev_(NULL)\n> > +{\n> > +}\n> \n> Same question as other patches. Empty constructors with only memeber\n> initializer list might go in the header file?\n\nSame answer :-) Why split it if there is one or more method implemented \nin the cpp file? I'm happy to do it in a standardized way just want to \nraise the question.\n\n> \n> > +\n> > +DeviceEnumeratorUdev::~DeviceEnumeratorUdev()\n> > +{\n> > +\tif (udev_)\n> > +\t\tudev_unref(udev_);\n> > +}\n> > +\n> > +int DeviceEnumeratorUdev::init()\n> > +{\n> > +\tif (udev_)\n> > +\t\treturn -EBUSY;\n> > +\n> > +\tudev_ = udev_new();\n> > +\tif (!udev_)\n> > +\t\treturn -ENODEV;\n> > +\n> > +\treturn 0;\n> > +}\n> > +\n> > +int DeviceEnumeratorUdev::enumerate()\n> > +{\n> > +\tstruct udev_enumerate *udev_enum = NULL;\n> > +\tstruct udev_list_entry *ents, *ent;\n> > +\tint ret;\n> > +\n> > +\tudev_enum = udev_enumerate_new(udev_);\n> > +\tif (udev_enum == NULL)\n> > +\t\treturn -ENOMEM;\n> > +\n> > +\tret = udev_enumerate_add_match_subsystem(udev_enum, \"media\");\n> > +\tif (ret < 0)\n> > +\t\tgoto done;\n> > +\n> > +\tret = udev_enumerate_scan_devices(udev_enum);\n> > +\tif (ret < 0)\n> > +\t\tgoto done;\n> > +\n> > +\tents = udev_enumerate_get_list_entry(udev_enum);\n> > +\tif (ents == NULL)\n> > +\t\tgoto done;\n> > +\n> > +\tudev_list_entry_foreach(ent, ents) {\n> > +\t\tstruct udev_device *dev;\n> > +\t\tconst char *devnode;\n> > +\t\tconst char *syspath = udev_list_entry_get_name(ent);\n> \n> nit: you've sorted variables in reverse-xmas-tree order in this file,\n> could you re-sort here to keep them consistent?\n\nNo preference, I always put declaration and assignment at the end of the \nlist.\n\n> \n> > +\n> > +\t\tdev = udev_device_new_from_syspath(udev_, syspath);\n> > +\t\tif (dev == NULL) {\n> \n> nullptr, or better !dev\n> \n> > +\t\t\tLOG(Error) << \"Failed to device for '\" << syspath << \"', skipping\";\n> \n> break to stay in 80 cols. We can span to 120 if it makes code more\n> clear, but here it is not necessary\n> \n> \t\t\tLOG(Error) << \"Failed to device for '\"\n> << syspath << \"', skipping\";\n> \n> Also \"Failed to device\": seems like you've missed the verb :)\n\nI agree I missed the verb ;-P\n\n\n> \n> > +\t\t\tcontinue;\n> > +\t\t}\n> > +\n> > +\t\tdevnode = udev_device_get_devnode(dev);\n> > +\t\tif (devnode == NULL) {\n> \n> nullptr, or better !dev\n> \n> > +\t\t\tudev_device_unref(dev);\n> > +\t\t\tret = -ENODEV;\n> > +\t\t\tgoto done;\n> > +\t\t}\n> > +\n> > +\t\taddDevice(devnode);\n> > +\n> > +\t\tudev_device_unref(dev);\n> > +\t}\n> > +done:\n> > +\tudev_enumerate_unref(udev_enum);\n> > +\treturn ret >= 0 ? 0 : ret;\n> > +}\n> > +\n> > +int DeviceEnumeratorUdev::lookupDevnode(std::string &devnode, int major, int minor)\n> > +{\n> > +\tstruct udev_device *device;\n> > +\tconst char *name;\n> > +\tdev_t devnum;\n> \n> This might be discussed as well, and I pointed it out in review's of\n> Kieran's series as well. I probably like more declaring variables at the\n> beginning of the function, but it's common practice to declare them\n> at initialization if possible.\n> \n> What do you think? Have a look at my series, where I tried to do that\n> consistently to make yourself an idea of how it would look like.\n\nNo opinion, lets reflect the outcome in the style document.\n\n> \n> > +\tint ret = 0;\n> > +\n> > +\tdevnum = makedev(major, minor);\n> > +\tdevice = udev_device_new_from_devnum(udev_, 'c', devnum);\n> > +\tif (!device)\n> > +\t\treturn -ENODEV;\n> > +\n> > +\tname = udev_device_get_devnode(device);\n> > +\tif (name)\n> > +\t\tdevnode = name;\n> > +\telse\n> > +\t\tret = -ENODEV;\n> > +\n> > +\tudev_device_unref(device);\n> > +\n> > +\treturn ret;\n> > +}\n> > +\n> > } /* namespace libcamera */\n> > diff --git a/src/libcamera/include/deviceenumerator.h b/src/libcamera/include/deviceenumerator.h\n> > index cbe17774edb7fcc5..2c7ff3f336ba127d 100644\n> > --- a/src/libcamera/include/deviceenumerator.h\n> > +++ b/src/libcamera/include/deviceenumerator.h\n> > @@ -78,6 +78,21 @@ private:\n> > \tvirtual int lookupDevnode(std::string &devnode, int major, int minor) = 0;\n> > };\n> >\n> > +class DeviceEnumeratorUdev: public DeviceEnumerator\n> > +{\n> > +public:\n> > +\tDeviceEnumeratorUdev();\n> > +\t~DeviceEnumeratorUdev();\n> > +\n> > +\tint init() final;\n> > +\tint enumerate() final;\n> > +\n> > +private:\n> > +\tstruct udev *udev_;\n> > +\n> > +\tint lookupDevnode(std::string &devnode, int major, int minor) final;\n> \n> This is what I was referring to in my comment to [00/12].\n> Here it's easy to have udev enumerate entities, and in the same class\n> use udev to lookup for devnode paths.\n> \n> If we go for the MediaDevice doing the entities enumeration, we need a\n> way to provide a method for doing devnode lookup using different\n> backend.\n\nI agree, that's why I think this method is cleaner.\n\n> \n> > +};\n> \n> The use of final applied to all methods declared virtual in the base\n> class makes me wonder if the whole class shouldn't be declared final.\n> But at the same time I wonder if we're sure this will be final for\n> real :)\n> \n> If not or the design thing, all minors comment :)\n> \n> Thanks\n> j\n> \n> > +\n> > } /* namespace libcamera */\n> >\n> > #endif\t/* __LIBCAMERA_DEVICEENUMERATE_H__ */\n> > --\n> > 2.20.1\n> >\n> > _______________________________________________\n> > libcamera-devel mailing list\n> > libcamera-devel@lists.libcamera.org\n> > https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"","Received":["from mail-lj1-x243.google.com (mail-lj1-x243.google.com\n\t[IPv6:2a00:1450:4864:20::243])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 21A64600CC\n\tfor ;\n\tFri, 28 Dec 2018 04:31:53 +0100 (CET)","by mail-lj1-x243.google.com with SMTP id t9-v6so17710931ljh.6\n\tfor ;\n\tThu, 27 Dec 2018 19:31:53 -0800 (PST)","from localhost (89-233-230-99.cust.bredband2.com. [89.233.230.99])\n\tby smtp.gmail.com with ESMTPSA id\n\ty24sm7645586lfj.17.2018.12.27.19.31.51\n\t(version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);\n\tThu, 27 Dec 2018 19:31:51 -0800 (PST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to\n\t:user-agent; bh=UFlCNXOlwbaix+FTIv4/+iEuoAyJqhQfnhIMQlh0nFU=;\n\tb=PwvBG7Jj/8zE8KWs/n2+8xyj5pfHMII+uEsKe/4eZky675Xc/EGOdIMOxIoUmby4Kk\n\tEAUpTR/B7UXIr/Qj1NACMkIJJzmlkpCAjMeXQrIdv6gw0wS36aOSrq5FglxGei4B8gMy\n\td+LMHBxIPrkEROsX4swSDogJMzXliZvmwfZDGvXDneDUMZXalXLwXsRWzUV0upKAtEcS\n\tHu7OOk4LytHECzMbftPhCh2xfbwBPuRtaBR8U0AZW0mTHO1ZgInhkvcZcyD3/zJfTa8y\n\tECNluDWwr+WqMkOTsgPpEnKcn6rznunl7IXztPAZFrGuJN15i1v7vn8Zd6TmgyqJPZ+v\n\trAcw==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to:user-agent;\n\tbh=UFlCNXOlwbaix+FTIv4/+iEuoAyJqhQfnhIMQlh0nFU=;\n\tb=Gv98g3+XYjSFbyCFf4yszakRUk3yrNU9RsYep7TXsulYcZgczzjh9uvJufTp7nx5Bh\n\teSStdwALIe4ysStufHJ+Pnv+X4wM34bwJa6ZMsUyZiozzqCvyz3eoFcHRtFUE7GCtl2S\n\tFMbcDoY8GH1fZZkdaiqaAwFerVyJIj3KUxFirbQeylGeh/Y8IGoL1cQT3+DH/GI5TRPJ\n\tPm0cpDMjcYfcJadNf2tnIw61hqoOOtRpywfc3sVrOoIHigZvbxpN/sdn5mezz4oT5tGh\n\tgJflBIq4aUsdessaq+G8jYAJlzYN8iL8bUAj2s1KPCgi55pEuO2giNpiqS0LIJJNiyJ/\n\tBaRQ==","X-Gm-Message-State":"AJcUukcFzYt5C5j+5dWCKl3I4Fk9kQXHX5cAidtwpnwid05Kc6MGb1UT\n\tOWPWg1DUO6LuivnVZDx4Gc8ztA==","X-Google-Smtp-Source":"ALg8bN6zbfwhX99HOgKG2l+ERethrQdL5cGr8lojIkPRZBbqFs+1mB+D8/wPzPOfJztCTj5s0o5kdw==","X-Received":"by 2002:a2e:81a:: with SMTP id\n\t26-v6mr16329780lji.14.1545967912399; \n\tThu, 27 Dec 2018 19:31:52 -0800 (PST)","Date":"Fri, 28 Dec 2018 04:31:51 +0100","From":"Niklas S??derlund ","To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20181228033151.GS19796@bigcity.dyn.berto.se>","References":"<20181222230041.29999-1-niklas.soderlund@ragnatech.se>\n\t<20181222230041.29999-7-niklas.soderlund@ragnatech.se>\n\t<20181224114049.GF2364@archlinux>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20181224114049.GF2364@archlinux>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH 06/12] libcamera: deviceenumerator:\n\tadd DeviceEnumeratorUdev class","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Fri, 28 Dec 2018 03:31:53 -0000"}},{"id":107,"web_url":"https://patchwork.libcamera.org/comment/107/","msgid":"<2706931.S25qMq79dR@avalon>","date":"2018-12-28T23:23:50","subject":"Re: [libcamera-devel] [PATCH 06/12] libcamera: deviceenumerator:\n\tadd DeviceEnumeratorUdev class","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hello,\n\nOn Friday, 28 December 2018 05:31:51 EET Niklas S??derlund wrote:\n> On 2018-12-24 12:40:49 +0100, Jacopo Mondi wrote:\n> > On Sun, Dec 23, 2018 at 12:00:35AM +0100, Niklas S??derlund wrote:\n> > > Provide a DeviceEnumeratorUdev class which is a specialization\n> > > of DeviceEnumerator which uses udev to enumerate information in the\n> > > system.\n> > > \n> > > Signed-off-by: Niklas Söderlund \n> > > ---\n> > > \n> > > src/libcamera/deviceenumerator.cpp | 100 +++++++++++++++++++++++\n> > > src/libcamera/include/deviceenumerator.h | 15 ++++\n> > > 2 files changed, 115 insertions(+)\n> > > \n> > > diff --git a/src/libcamera/deviceenumerator.cpp\n> > > b/src/libcamera/deviceenumerator.cpp index\n> > > b2f59017e94d14aa..f4c40bf0376ab453 100644\n> > > --- a/src/libcamera/deviceenumerator.cpp\n> > > +++ b/src/libcamera/deviceenumerator.cpp\n> > > @@ -6,6 +6,7 @@\n> > > */\n> > > \n> > > #include \n> > > +#include \n> > > #include \n> > > #include \n> > > \n> > > @@ -273,4 +274,103 @@ DeviceInfo *DeviceEnumerator::search(DeviceMatch\n> > > &dm) const\n> > > \treturn info;\n> > > }\n> > > \n> > > +/* --------------------------------------------------------------------\n> > > + * Enumerator Udev\n> > > + */\n> > > +\n> > > +DeviceEnumeratorUdev::DeviceEnumeratorUdev()\n> > > +\t: udev_(NULL)\n\nnullptr (same through the whole series).\n\n> > > +{\n> > > +}\n> > \n> > Same question as other patches. Empty constructors with only memeber\n> > initializer list might go in the header file?\n> \n> Same answer :-) Why split it if there is one or more method implemented\n> in the cpp file? I'm happy to do it in a standardized way just want to\n> raise the question.\n> \n> > > +\n> > > +DeviceEnumeratorUdev::~DeviceEnumeratorUdev()\n> > > +{\n> > > +\tif (udev_)\n> > > +\t\tudev_unref(udev_);\n> > > +}\n> > > +\n> > > +int DeviceEnumeratorUdev::init()\n> > > +{\n> > > +\tif (udev_)\n> > > +\t\treturn -EBUSY;\n> > > +\n> > > +\tudev_ = udev_new();\n> > > +\tif (!udev_)\n> > > +\t\treturn -ENODEV;\n> > > +\n> > > +\treturn 0;\n> > > +}\n> > > +\n> > > +int DeviceEnumeratorUdev::enumerate()\n> > > +{\n> > > +\tstruct udev_enumerate *udev_enum = NULL;\n> > > +\tstruct udev_list_entry *ents, *ent;\n> > > +\tint ret;\n> > > +\n> > > +\tudev_enum = udev_enumerate_new(udev_);\n> > > +\tif (udev_enum == NULL)\n> > > +\t\treturn -ENOMEM;\n> > > +\n> > > +\tret = udev_enumerate_add_match_subsystem(udev_enum, \"media\");\n> > > +\tif (ret < 0)\n> > > +\t\tgoto done;\n> > > +\n> > > +\tret = udev_enumerate_scan_devices(udev_enum);\n> > > +\tif (ret < 0)\n> > > +\t\tgoto done;\n> > > +\n> > > +\tents = udev_enumerate_get_list_entry(udev_enum);\n> > > +\tif (ents == NULL)\n> > > +\t\tgoto done;\n> > > +\n> > > +\tudev_list_entry_foreach(ent, ents) {\n> > > +\t\tstruct udev_device *dev;\n> > > +\t\tconst char *devnode;\n> > > +\t\tconst char *syspath = udev_list_entry_get_name(ent);\n> > \n> > nit: you've sorted variables in reverse-xmas-tree order in this file,\n> > could you re-sort here to keep them consistent?\n> \n> No preference, I always put declaration and assignment at the end of the\n> list.\n> \n> > > +\n> > > +\t\tdev = udev_device_new_from_syspath(udev_, syspath);\n> > > +\t\tif (dev == NULL) {\n> > \n> > nullptr, or better !dev\n> > \n> > > +\t\t\tLOG(Error) << \"Failed to device for '\" << syspath << \"', \nskipping\";\n> > \n> > break to stay in 80 cols. We can span to 120 if it makes code more\n> > clear, but here it is not necessary\n> > \n> > \t\t\tLOG(Error) << \"Failed to device for '\"\n> > \t\t\t\n> > << syspath << \"', skipping\";\n> > \n> > Also \"Failed to device\": seems like you've missed the verb :)\n> \n> I agree I missed the verb ;-P\n> \n> > > +\t\t\tcontinue;\n> > > +\t\t}\n> > > +\n> > > +\t\tdevnode = udev_device_get_devnode(dev);\n> > > +\t\tif (devnode == NULL) {\n> > \n> > nullptr, or better !dev\n> > \n> > > +\t\t\tudev_device_unref(dev);\n> > > +\t\t\tret = -ENODEV;\n> > > +\t\t\tgoto done;\n> > > +\t\t}\n> > > +\n> > > +\t\taddDevice(devnode);\n> > > +\n> > > +\t\tudev_device_unref(dev);\n> > > +\t}\n> > > +done:\n> > > +\tudev_enumerate_unref(udev_enum);\n> > > +\treturn ret >= 0 ? 0 : ret;\n> > > +}\n> > > +\n> > > +int DeviceEnumeratorUdev::lookupDevnode(std::string &devnode, int\n> > > major, int minor)\n\nHow about listing output variables after input variables ?\n\n> > > +{\n> > > +\tstruct udev_device *device;\n> > > +\tconst char *name;\n> > > +\tdev_t devnum;\n> > \n> > This might be discussed as well, and I pointed it out in review's of\n> > Kieran's series as well. I probably like more declaring variables at the\n> > beginning of the function, but it's common practice to declare them\n> > at initialization if possible.\n> > \n> > What do you think? Have a look at my series, where I tried to do that\n> > consistently to make yourself an idea of how it would look like.\n> \n> No opinion, lets reflect the outcome in the style document.\n\nI think it makes sense to group variable declaration and code when they're \nrelated, but for variables reused through the function (such as ret) I find \ndeclaring them at the beginning of the function better than at the location of \ntheir first use.\n\n> > > +\tint ret = 0;\n> > > +\n> > > +\tdevnum = makedev(major, minor);\n> > > +\tdevice = udev_device_new_from_devnum(udev_, 'c', devnum);\n> > > +\tif (!device)\n> > > +\t\treturn -ENODEV;\n> > > +\n> > > +\tname = udev_device_get_devnode(device);\n> > > +\tif (name)\n> > > +\t\tdevnode = name;\n> > > +\telse\n> > > +\t\tret = -ENODEV;\n> > > +\n> > > +\tudev_device_unref(device);\n> > > +\n> > > +\treturn ret;\n\nYou could also return an empty string\n\n> > > +}\n> > > +\n> > > \n> > > } /* namespace libcamera */\n> > > \n> > > diff --git a/src/libcamera/include/deviceenumerator.h\n> > > b/src/libcamera/include/deviceenumerator.h index\n> > > cbe17774edb7fcc5..2c7ff3f336ba127d 100644\n> > > --- a/src/libcamera/include/deviceenumerator.h\n> > > +++ b/src/libcamera/include/deviceenumerator.h\n> > > \n> > > @@ -78,6 +78,21 @@ private:\n> > > \tvirtual int lookupDevnode(std::string &devnode, int major, int \nminor)\n> > > \t= 0;\n> > > };\n> > > \n> > > +class DeviceEnumeratorUdev: public DeviceEnumerator\n> > > +{\n> > > +public:\n> > > +\tDeviceEnumeratorUdev();\n> > > +\t~DeviceEnumeratorUdev();\n> > > +\n> > > +\tint init() final;\n> > > +\tint enumerate() final;\n> > > +\n> > > +private:\n> > > +\tstruct udev *udev_;\n> > > +\n> > > +\tint lookupDevnode(std::string &devnode, int major, int minor) final;\n> > \n> > This is what I was referring to in my comment to [00/12].\n> > Here it's easy to have udev enumerate entities, and in the same class\n> > use udev to lookup for devnode paths.\n> > \n> > If we go for the MediaDevice doing the entities enumeration, we need a\n> > way to provide a method for doing devnode lookup using different\n> > backend.\n> \n> I agree, that's why I think this method is cleaner.\n> \n> > > +};\n> > \n> > The use of final applied to all methods declared virtual in the base\n> > class makes me wonder if the whole class shouldn't be declared final.\n> > But at the same time I wonder if we're sure this will be final for\n> > real :)\n\nIt's an internal API so it can always change later :-)\n\n> > If not or the design thing, all minors comment :)\n> > \n> > > +\n> > > \n> > > } /* namespace libcamera */\n> > > \n> > > #endif\t/* __LIBCAMERA_DEVICEENUMERATE_H__ */","headers":{"Return-Path":"","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B2A5760B2E\n\tfor ;\n\tSat, 29 Dec 2018 00:22:54 +0100 (CET)","from avalon.localnet (unknown\n\t[IPv6:2a02:2788:66a:3eb:2624:a446:f4b7:b19d])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 3C699DF;\n\tSat, 29 Dec 2018 00:22:54 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1546039374;\n\tbh=/B5P9YBi7NDHCWl2rXvgNV3f2gSwNcBtil6LQNcubK8=;\n\th=From:To:Cc:Subject:Date:In-Reply-To:References:From;\n\tb=mtt+Hu/Ow6wtpxDI512wOhD/aQJovsLd5U+tp+VrQgseHITyZt+iB7DU5nQzYZUiB\n\tZ57wTsRGDPyY3KZIxcTa3QMBtBbeaqLXL0mV7cddvPUlrV5+jD5mNwOgt+MjARshXN\n\to63ZGnJq3yNX3W56WJQ6LAiySJDNipFsObusmK44=","From":"Laurent Pinchart ","To":"libcamera-devel@lists.libcamera.org","Date":"Sat, 29 Dec 2018 01:23:50 +0200","Message-ID":"<2706931.S25qMq79dR@avalon>","Organization":"Ideas on Board Oy","In-Reply-To":"<20181228033151.GS19796@bigcity.dyn.berto.se>","References":"<20181222230041.29999-1-niklas.soderlund@ragnatech.se>\n\t<20181224114049.GF2364@archlinux>\n\t<20181228033151.GS19796@bigcity.dyn.berto.se>","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","Content-Type":"text/plain; charset=\"iso-8859-1\"","Subject":"Re: [libcamera-devel] [PATCH 06/12] libcamera: deviceenumerator:\n\tadd DeviceEnumeratorUdev class","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Fri, 28 Dec 2018 23:22:54 -0000"}},{"id":117,"web_url":"https://patchwork.libcamera.org/comment/117/","msgid":"<20181229015415.GC19796@bigcity.dyn.berto.se>","date":"2018-12-29T01:54:15","subject":"Re: [libcamera-devel] [PATCH 06/12] libcamera: deviceenumerator:\n\tadd DeviceEnumeratorUdev class","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Laurent,\n\nThanks for your comments.\n\nOn 2018-12-29 01:23:50 +0200, Laurent Pinchart wrote:\n> Hello,\n> \n> On Friday, 28 December 2018 05:31:51 EET Niklas S??derlund wrote:\n> > On 2018-12-24 12:40:49 +0100, Jacopo Mondi wrote:\n> > > On Sun, Dec 23, 2018 at 12:00:35AM +0100, Niklas S??derlund wrote:\n> > > > Provide a DeviceEnumeratorUdev class which is a specialization\n> > > > of DeviceEnumerator which uses udev to enumerate information in the\n> > > > system.\n> > > > \n> > > > Signed-off-by: Niklas Söderlund \n> > > > ---\n> > > > \n> > > > src/libcamera/deviceenumerator.cpp | 100 +++++++++++++++++++++++\n> > > > src/libcamera/include/deviceenumerator.h | 15 ++++\n> > > > 2 files changed, 115 insertions(+)\n> > > > \n> > > > diff --git a/src/libcamera/deviceenumerator.cpp\n> > > > b/src/libcamera/deviceenumerator.cpp index\n> > > > b2f59017e94d14aa..f4c40bf0376ab453 100644\n> > > > --- a/src/libcamera/deviceenumerator.cpp\n> > > > +++ b/src/libcamera/deviceenumerator.cpp\n> > > > @@ -6,6 +6,7 @@\n> > > > */\n> > > > \n> > > > #include \n> > > > +#include \n> > > > #include \n> > > > #include \n> > > > \n> > > > @@ -273,4 +274,103 @@ DeviceInfo *DeviceEnumerator::search(DeviceMatch\n> > > > &dm) const\n> > > > \treturn info;\n> > > > }\n> > > > \n> > > > +/* --------------------------------------------------------------------\n> > > > + * Enumerator Udev\n> > > > + */\n> > > > +\n> > > > +DeviceEnumeratorUdev::DeviceEnumeratorUdev()\n> > > > +\t: udev_(NULL)\n> \n> nullptr (same through the whole series).\n\nThanks.\n\n> \n> > > > +{\n> > > > +}\n> > > \n> > > Same question as other patches. Empty constructors with only memeber\n> > > initializer list might go in the header file?\n> > \n> > Same answer :-) Why split it if there is one or more method implemented\n> > in the cpp file? I'm happy to do it in a standardized way just want to\n> > raise the question.\n> > \n> > > > +\n> > > > +DeviceEnumeratorUdev::~DeviceEnumeratorUdev()\n> > > > +{\n> > > > +\tif (udev_)\n> > > > +\t\tudev_unref(udev_);\n> > > > +}\n> > > > +\n> > > > +int DeviceEnumeratorUdev::init()\n> > > > +{\n> > > > +\tif (udev_)\n> > > > +\t\treturn -EBUSY;\n> > > > +\n> > > > +\tudev_ = udev_new();\n> > > > +\tif (!udev_)\n> > > > +\t\treturn -ENODEV;\n> > > > +\n> > > > +\treturn 0;\n> > > > +}\n> > > > +\n> > > > +int DeviceEnumeratorUdev::enumerate()\n> > > > +{\n> > > > +\tstruct udev_enumerate *udev_enum = NULL;\n> > > > +\tstruct udev_list_entry *ents, *ent;\n> > > > +\tint ret;\n> > > > +\n> > > > +\tudev_enum = udev_enumerate_new(udev_);\n> > > > +\tif (udev_enum == NULL)\n> > > > +\t\treturn -ENOMEM;\n> > > > +\n> > > > +\tret = udev_enumerate_add_match_subsystem(udev_enum, \"media\");\n> > > > +\tif (ret < 0)\n> > > > +\t\tgoto done;\n> > > > +\n> > > > +\tret = udev_enumerate_scan_devices(udev_enum);\n> > > > +\tif (ret < 0)\n> > > > +\t\tgoto done;\n> > > > +\n> > > > +\tents = udev_enumerate_get_list_entry(udev_enum);\n> > > > +\tif (ents == NULL)\n> > > > +\t\tgoto done;\n> > > > +\n> > > > +\tudev_list_entry_foreach(ent, ents) {\n> > > > +\t\tstruct udev_device *dev;\n> > > > +\t\tconst char *devnode;\n> > > > +\t\tconst char *syspath = udev_list_entry_get_name(ent);\n> > > \n> > > nit: you've sorted variables in reverse-xmas-tree order in this file,\n> > > could you re-sort here to keep them consistent?\n> > \n> > No preference, I always put declaration and assignment at the end of the\n> > list.\n> > \n> > > > +\n> > > > +\t\tdev = udev_device_new_from_syspath(udev_, syspath);\n> > > > +\t\tif (dev == NULL) {\n> > > \n> > > nullptr, or better !dev\n> > > \n> > > > +\t\t\tLOG(Error) << \"Failed to device for '\" << syspath << \"', \n> skipping\";\n> > > \n> > > break to stay in 80 cols. We can span to 120 if it makes code more\n> > > clear, but here it is not necessary\n> > > \n> > > \t\t\tLOG(Error) << \"Failed to device for '\"\n> > > \t\t\t\n> > > << syspath << \"', skipping\";\n> > > \n> > > Also \"Failed to device\": seems like you've missed the verb :)\n> > \n> > I agree I missed the verb ;-P\n> > \n> > > > +\t\t\tcontinue;\n> > > > +\t\t}\n> > > > +\n> > > > +\t\tdevnode = udev_device_get_devnode(dev);\n> > > > +\t\tif (devnode == NULL) {\n> > > \n> > > nullptr, or better !dev\n> > > \n> > > > +\t\t\tudev_device_unref(dev);\n> > > > +\t\t\tret = -ENODEV;\n> > > > +\t\t\tgoto done;\n> > > > +\t\t}\n> > > > +\n> > > > +\t\taddDevice(devnode);\n> > > > +\n> > > > +\t\tudev_device_unref(dev);\n> > > > +\t}\n> > > > +done:\n> > > > +\tudev_enumerate_unref(udev_enum);\n> > > > +\treturn ret >= 0 ? 0 : ret;\n> > > > +}\n> > > > +\n> > > > +int DeviceEnumeratorUdev::lookupDevnode(std::string &devnode, int\n> > > > major, int minor)\n> \n> How about listing output variables after input variables ?\n> \n> > > > +{\n> > > > +\tstruct udev_device *device;\n> > > > +\tconst char *name;\n> > > > +\tdev_t devnum;\n> > > \n> > > This might be discussed as well, and I pointed it out in review's of\n> > > Kieran's series as well. I probably like more declaring variables at the\n> > > beginning of the function, but it's common practice to declare them\n> > > at initialization if possible.\n> > > \n> > > What do you think? Have a look at my series, where I tried to do that\n> > > consistently to make yourself an idea of how it would look like.\n> > \n> > No opinion, lets reflect the outcome in the style document.\n> \n> I think it makes sense to group variable declaration and code when they're \n> related, but for variables reused through the function (such as ret) I find \n> declaring them at the beginning of the function better than at the location of \n> their first use.\n> \n> > > > +\tint ret = 0;\n> > > > +\n> > > > +\tdevnum = makedev(major, minor);\n> > > > +\tdevice = udev_device_new_from_devnum(udev_, 'c', devnum);\n> > > > +\tif (!device)\n> > > > +\t\treturn -ENODEV;\n> > > > +\n> > > > +\tname = udev_device_get_devnode(device);\n> > > > +\tif (name)\n> > > > +\t\tdevnode = name;\n> > > > +\telse\n> > > > +\t\tret = -ENODEV;\n> > > > +\n> > > > +\tudev_device_unref(device);\n> > > > +\n> > > > +\treturn ret;\n> \n> You could also return an empty string\n\nI went with this approach.\n\n> \n> > > > +}\n> > > > +\n> > > > \n> > > > } /* namespace libcamera */\n> > > > \n> > > > diff --git a/src/libcamera/include/deviceenumerator.h\n> > > > b/src/libcamera/include/deviceenumerator.h index\n> > > > cbe17774edb7fcc5..2c7ff3f336ba127d 100644\n> > > > --- a/src/libcamera/include/deviceenumerator.h\n> > > > +++ b/src/libcamera/include/deviceenumerator.h\n> > > > \n> > > > @@ -78,6 +78,21 @@ private:\n> > > > \tvirtual int lookupDevnode(std::string &devnode, int major, int \n> minor)\n> > > > \t= 0;\n> > > > };\n> > > > \n> > > > +class DeviceEnumeratorUdev: public DeviceEnumerator\n> > > > +{\n> > > > +public:\n> > > > +\tDeviceEnumeratorUdev();\n> > > > +\t~DeviceEnumeratorUdev();\n> > > > +\n> > > > +\tint init() final;\n> > > > +\tint enumerate() final;\n> > > > +\n> > > > +private:\n> > > > +\tstruct udev *udev_;\n> > > > +\n> > > > +\tint lookupDevnode(std::string &devnode, int major, int minor) final;\n> > > \n> > > This is what I was referring to in my comment to [00/12].\n> > > Here it's easy to have udev enumerate entities, and in the same class\n> > > use udev to lookup for devnode paths.\n> > > \n> > > If we go for the MediaDevice doing the entities enumeration, we need a\n> > > way to provide a method for doing devnode lookup using different\n> > > backend.\n> > \n> > I agree, that's why I think this method is cleaner.\n> > \n> > > > +};\n> > > \n> > > The use of final applied to all methods declared virtual in the base\n> > > class makes me wonder if the whole class shouldn't be declared final.\n> > > But at the same time I wonder if we're sure this will be final for\n> > > real :)\n> \n> It's an internal API so it can always change later :-)\n> \n> > > If not or the design thing, all minors comment :)\n> > > \n> > > > +\n> > > > \n> > > > } /* namespace libcamera */\n> > > > \n> > > > #endif\t/* __LIBCAMERA_DEVICEENUMERATE_H__ */\n> \n> -- \n> Regards,\n> \n> Laurent Pinchart\n> \n> \n>","headers":{"Return-Path":"","Received":["from mail-lf1-x141.google.com (mail-lf1-x141.google.com\n\t[IPv6:2a00:1450:4864:20::141])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1893B60B30\n\tfor ;\n\tSat, 29 Dec 2018 02:54:18 +0100 (CET)","by mail-lf1-x141.google.com with SMTP id p86so15496441lfg.5\n\tfor ;\n\tFri, 28 Dec 2018 17:54:18 -0800 (PST)","from localhost (89-233-230-99.cust.bredband2.com. [89.233.230.99])\n\tby smtp.gmail.com with ESMTPSA id\n\tk14sm8212049lfc.70.2018.12.28.17.54.15\n\t(version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256);\n\tFri, 28 Dec 2018 17:54:16 -0800 (PST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to\n\t:user-agent; bh=Aauvz5x5bR5yRrXYLpE1W6fFi5RR47KxANbSKwYTEXY=;\n\tb=cDWjWghx9kYoCkK76ZwOfs1JRacxC/gYFCDdKpNSZEBkvmG67VFWYJJAGLI9xSTHuM\n\taz9rgDMimbf209MaOBH2ViBe3aH45BolCvcBLL22PWhkxNwYelhOhluOMVY2wCAPXG1Q\n\tQgVw1MvrYgxRbUBDcdlFxlwIT5JGDKS6vaJemsD8AV5SHobWJP8w7foMa2zN5DpQBYsE\n\tLHxs1MT8/n5b/hwKQMQ/cI5TvPf+seHNBwuqIMXalp8adNxCe876F44GU2DfQASLoNLK\n\ta4nc+6DR6W1ye1/geCPrxYk5lNgPJqGdoe0NGtdRCqIizjMYWDt8XOIsNzIovjO2raib\n\tZr/g==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to:user-agent;\n\tbh=Aauvz5x5bR5yRrXYLpE1W6fFi5RR47KxANbSKwYTEXY=;\n\tb=DH4qnviOWYTlWe8x6/VRyacF/+jUs0awYBaph3TrweQM6NvYmMREfYd3lEzWGKEpbF\n\t+mVUt3Vxmk67KFOmSqsC/b/w6mgTiQ+TrHhb2DSzodL8fdQWST7F/M8eLz09ltdOrqpH\n\tTtr+h+oUXbb95delu237gjiM2Vu/zRUTv+rQLXA705FwTZ8l+XZjpZi2Dl9Uhf3xAwz8\n\t/5lGgiHcmA0O3lXINSATl3aFF9iyNO8fZzWejz02kszmeNAdeh1k8uhD5VAE0N9ZdnBC\n\t5Z51IlkMqy5xFatpdARJBQ5tPgd4uwG3nfVr9d+oJ1Ma4ZXB0NGMCY7TVuasaICyPg8/\n\tWteg==","X-Gm-Message-State":"AA+aEWbk6AvoKMU2UFh3PQBKM3UA7Nr5xfXSbuNPhIi4dQUaTNWXjwMv\n\tRI/LgD/vI5RgchiBej1EGi8Pv1mtt5o=","X-Google-Smtp-Source":"AFSGD/UTMvcYirI5RtNNQclY4OTQEpaWWxf7mLy1Rk6TGzeqCpVJidUYIMolG2TvCTHB49jd/oZnIQ==","X-Received":"by 2002:a19:5059:: with SMTP id\n\tz25mr14663902lfj.120.1546048457352; \n\tFri, 28 Dec 2018 17:54:17 -0800 (PST)","Date":"Sat, 29 Dec 2018 02:54:15 +0100","From":"Niklas S??derlund ","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org, Jacopo Mondi ","Message-ID":"<20181229015415.GC19796@bigcity.dyn.berto.se>","References":"<20181222230041.29999-1-niklas.soderlund@ragnatech.se>\n\t<20181224114049.GF2364@archlinux>\n\t<20181228033151.GS19796@bigcity.dyn.berto.se>\n\t<2706931.S25qMq79dR@avalon>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<2706931.S25qMq79dR@avalon>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH 06/12] libcamera: deviceenumerator:\n\tadd DeviceEnumeratorUdev class","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Sat, 29 Dec 2018 01:54:18 -0000"}}]