Received-SPF: pass (google.com: domain of laurent.pinchart@ideasonboard.com designates 213.167.242.64 as permitted sender) client-ip=213.167.242.64;
From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: libcamera-devel@lists.libcamera.org
Cc: Khem Raj <raj.khem@gmail.com>
Subject: [PATCH v2] libcamera: pipeline: uvcvideo: Avoid reference to temporary object
Date: Sun, 28 Mar 2021 05:57:41 +0300
Message-Id: <20210328025741.30246-1-laurent.pinchart@ideasonboard.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit

From: Khem Raj <raj.khem@gmail.com>

A range-based for loop whose range expression is an array of char
pointers and range variable declaration is a const reference to a
std::string creates a temporary string from the char pointer and binds
the range variable reference to it. This creates a const reference to a
temporary, which is valid in C++, and extends the lifetime of the
temporary to the lifetime of the reference.

However, lifetime extension in range-based for loops is considered as a
sign of a potential issue, as a temporary is created for every
iteration, which can be costly, and the usage of a reference in the
range declaration doesn't make it obvious that the code isn't simply
binding a reference to an existing object. gcc 11, with the
-Wrange-loop-construct option, flags 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" }) {
|       |                                 ^~~~

To please the compiler, make the range variable a const char *. This may
bring a tiny performance improvement, as the name is only used once, in
a location where the compiler can use

	operator+(const std::string &, const char *)

instead of

	operator+(const std::string &, const std::string &)

Signed-off-by: Khem Raj <raj.khem@gmail.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Use a const char * type instead of auto, and update the commit message
accordingly.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.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 031f96e28449..b6c6ade5ebaf 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 char *name : { "idVendor", "idProduct" }) {
 		std::ifstream file(path + "/../" + name);
 
 		if (!file.is_open())
-- 
Regards,

Laurent Pinchart