[v1] apps: common: options: Avoid copying in range based for loop
diff mbox series

Message ID 20250814151420.2119927-1-barnabas.pocze@ideasonboard.com
State Accepted
Headers show
Series
  • [v1] apps: common: options: Avoid copying in range based for loop
Related show

Commit Message

Barnabás Pőcze Aug. 14, 2025, 3:14 p.m. UTC
The copy can trigger `-Wrange-loop-construct`, so use a reference
to avoid the warning.

Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
---
 src/apps/common/options.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Laurent Pinchart Aug. 14, 2025, 9:58 p.m. UTC | #1
On Thu, Aug 14, 2025 at 05:14:20PM +0200, Barnabás Pőcze wrote:
> The copy can trigger `-Wrange-loop-construct`, so use a reference
> to avoid the warning.

It's not just about the warning, using a reference avoids a copy, which
is also more efficient.

> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  src/apps/common/options.cpp | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/apps/common/options.cpp b/src/apps/common/options.cpp
> index cae193cc4..b4ea1afe2 100644
> --- a/src/apps/common/options.cpp
> +++ b/src/apps/common/options.cpp
> @@ -887,7 +887,7 @@ OptionsParser::Options OptionsParser::parse(int argc, char **argv)
>  
>  	shortOptions[ids++] = ':';
>  
> -	for (const auto [opt, option] : optionsMap_) {
> +	for (const auto &[opt, option] : optionsMap_) {
>  		if (option->hasShortOption()) {
>  			shortOptions[ids++] = opt;
>  			if (option->argument != ArgumentNone)
Kieran Bingham Aug. 15, 2025, 9:20 a.m. UTC | #2
Quoting Barnabás Pőcze (2025-08-14 16:14:20)
> The copy can trigger `-Wrange-loop-construct`, so use a reference
> to avoid the warning.
> 
> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>

Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

> ---
>  src/apps/common/options.cpp | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/apps/common/options.cpp b/src/apps/common/options.cpp
> index cae193cc4..b4ea1afe2 100644
> --- a/src/apps/common/options.cpp
> +++ b/src/apps/common/options.cpp
> @@ -887,7 +887,7 @@ OptionsParser::Options OptionsParser::parse(int argc, char **argv)
>  
>         shortOptions[ids++] = ':';
>  
> -       for (const auto [opt, option] : optionsMap_) {
> +       for (const auto &[opt, option] : optionsMap_) {
>                 if (option->hasShortOption()) {
>                         shortOptions[ids++] = opt;
>                         if (option->argument != ArgumentNone)
> -- 
> 2.50.1
>

Patch
diff mbox series

diff --git a/src/apps/common/options.cpp b/src/apps/common/options.cpp
index cae193cc4..b4ea1afe2 100644
--- a/src/apps/common/options.cpp
+++ b/src/apps/common/options.cpp
@@ -887,7 +887,7 @@  OptionsParser::Options OptionsParser::parse(int argc, char **argv)
 
 	shortOptions[ids++] = ':';
 
-	for (const auto [opt, option] : optionsMap_) {
+	for (const auto &[opt, option] : optionsMap_) {
 		if (option->hasShortOption()) {
 			shortOptions[ids++] = opt;
 			if (option->argument != ArgumentNone)