[libcamera-devel] [PATCH] uvcvideo: Use auto variable to avoid range loop warnings
Khem Raj
raj.khem at gmail.com
Tue Mar 9 01:13:57 CET 2021
On Mon, Mar 8, 2021 at 4:07 PM Laurent Pinchart
<laurent.pinchart at ideasonboard.com> wrote:
>
> Hi Khem,
>
> On Thu, Mar 04, 2021 at 02:00:37AM +0200, Laurent Pinchart wrote:
> > On Wed, Mar 03, 2021 at 03:21:26PM -0800, Khem Raj wrote:
> > > With c++17 loop range bases are defined where copy is obvious since
> > > iterator returns a copy and not reference, gcc11 will emit a warning
> > > about this
> > >
> > > uvcvideo.cpp:432:33: error: loop variable 'name' of type 'const string&' {aka 'const std::__cxx11::basic_string<cha
> > > r>&'} binds to a temporary constructed from type 'const char* const' [-Werror=range-loop-construct]
> > > | 432 | for (const std::string &name : { "idVendor", "idProduct" }) {
> > > | | ^~~~
> >
> > Looks like I should add gcc 11 to my build tests :-)
> >
> > > Therefore making it explicit is better
> > >
> > > Signed-off-by: Khem Raj <raj.khem at gmail.com>
> > > ---
> > > src/libcamera/pipeline/uvcvideo/uvcvideo.cpp | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> > > index 031f96e2..ef23ece7 100644
> > > --- a/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> > > +++ b/src/libcamera/pipeline/uvcvideo/uvcvideo.cpp
> > > @@ -429,7 +429,7 @@ std::string PipelineHandlerUVC::generateId(const UVCCameraData *data)
> > >
> > > /* Creata a device ID from the USB devices vendor and product ID. */
> > > std::string deviceId;
> > > - for (const std::string &name : { "idVendor", "idProduct" }) {
> > > + for (const auto name : { "idVendor", "idProduct" }) {
> >
> > What would you think about making this
> >
> > for (const char *name : { "idVendor", "idProduct" }) {
> >
> > ? We tend to only use auto when the explicit type is either impossible
> > to type, or would be too long, as an explicit type makes it easier for
> > the reader to know the type of the variable. I however have to say that
> > using auto here since the beginning would have prevented this bug from
> > happening in the first place. I'll let you decide what you think is
> > best, and if you opt for const char *name, I can change this when
> > applying, there's no need to resend the patch.
>
> Would this change be OK for you ?
>
I have to test this out and haven't yet got to it. but I think this
change will be ok as well.
> > Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> >
> > > std::ifstream file(path + "/../" + name);
> > >
> > > if (!file.is_open())
>
> --
> Regards,
>
> Laurent Pinchart
More information about the libcamera-devel
mailing list