[RFC,v3,2/9] libcamera: process: Misc. cleanup around `execv()`
diff mbox series

Message ID 20250325180821.1456154-3-barnabas.pocze@ideasonboard.com
State New
Headers show
Series
  • libcamera: process: Remove `ProcessManager` singleton
Related show

Commit Message

Barnabás Pőcze March 25, 2025, 6:08 p.m. UTC 
Firstly, get the number of arguments first, and use that to determine the
size of the allocation instead of retrieving it twice.

Secondly, use `const_cast` instead of a C-style cast when calling `execv()`.

Third, use `size_t` to match the type of `args.size()`.

Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
---
 src/libcamera/process.cpp | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

Comments

Laurent Pinchart March 26, 2025, 1:01 p.m. UTC | #1 
Hi Barnabás,

Thank you for the patch.

On Tue, Mar 25, 2025 at 07:08:14PM +0100, Barnabás Pőcze wrote:
> Firstly, get the number of arguments first, and use that to determine the
> size of the allocation instead of retrieving it twice.
> 
> Secondly, use `const_cast` instead of a C-style cast when calling `execv()`.
> 
> Third, use `size_t` to match the type of `args.size()`.
> 
> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
> ---
>  src/libcamera/process.cpp | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/src/libcamera/process.cpp b/src/libcamera/process.cpp
> index 68fad3270..567b878d4 100644
> --- a/src/libcamera/process.cpp
> +++ b/src/libcamera/process.cpp
> @@ -265,14 +265,15 @@ int Process::start(const std::string &path,
>  		if (file && strcmp(file, "syslog"))
>  			unsetenv("LIBCAMERA_LOG_FILE");
>  
> -		const char **argv = new const char *[args.size() + 2];
> -		unsigned int len = args.size();
> +		size_t len = args.size();

While at it,

		const size_t len = args.size();

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

> +		auto argv = std::make_unique<const char *[]>(len + 2);
> +
>  		argv[0] = path.c_str();
> -		for (unsigned int i = 0; i < len; i++)
> +		for (size_t i = 0; i < len; i++)
>  			argv[i + 1] = args[i].c_str();
>  		argv[len + 1] = nullptr;
>  
> -		execv(path.c_str(), (char **)argv);
> +		execv(path.c_str(), const_cast<char **>(argv.get()));
>  
>  		exit(EXIT_FAILURE);
>  	}

Patch
diff mbox series 

diff --git a/src/libcamera/process.cpp b/src/libcamera/process.cpp
index 68fad3270..567b878d4 100644
--- a/src/libcamera/process.cpp
+++ b/src/libcamera/process.cpp
@@ -265,14 +265,15 @@  int Process::start(const std::string &path,
 		if (file && strcmp(file, "syslog"))
 			unsetenv("LIBCAMERA_LOG_FILE");
 
-		const char **argv = new const char *[args.size() + 2];
-		unsigned int len = args.size();
+		size_t len = args.size();
+		auto argv = std::make_unique<const char *[]>(len + 2);
+
 		argv[0] = path.c_str();
-		for (unsigned int i = 0; i < len; i++)
+		for (size_t i = 0; i < len; i++)
 			argv[i + 1] = args[i].c_str();
 		argv[len + 1] = nullptr;
 
-		execv(path.c_str(), (char **)argv);
+		execv(path.c_str(), const_cast<char **>(argv.get()));
 
 		exit(EXIT_FAILURE);
 	}