[RFC,v3,6/9] libcamera: process: Use span instead of vector
diff mbox series

Message ID 20250325180821.1456154-7-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 
Use `libcamera::Span` whenever applicable.

Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
---
 include/libcamera/internal/process.h  | 8 ++++----
 src/libcamera/ipc_pipe_unixsocket.cpp | 9 +++------
 src/libcamera/process.cpp             | 8 ++++----
 test/process/process_test.cpp         | 5 ++---
 4 files changed, 13 insertions(+), 17 deletions(-)

Comments

Kieran Bingham March 25, 2025, 7:18 p.m. UTC | #1 
Quoting Barnabás Pőcze (2025-03-25 18:08:18)
> Use `libcamera::Span` whenever applicable.
> 
> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
> ---
>  include/libcamera/internal/process.h  | 8 ++++----
>  src/libcamera/ipc_pipe_unixsocket.cpp | 9 +++------

If you're in here, it might be worth reviewing another of Julien's
patches here too:

- https://patchwork.libcamera.org/patch/22425/

But I think this patch seems ok to me.


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

>  src/libcamera/process.cpp             | 8 ++++----
>  test/process/process_test.cpp         | 5 ++---
>  4 files changed, 13 insertions(+), 17 deletions(-)
> 
> diff --git a/include/libcamera/internal/process.h b/include/libcamera/internal/process.h
> index 030a1e66e..e4f5bb979 100644
> --- a/include/libcamera/internal/process.h
> +++ b/include/libcamera/internal/process.h
> @@ -9,9 +9,9 @@
>  
>  #include <signal.h>
>  #include <string>
> -#include <vector>
>  
>  #include <libcamera/base/signal.h>
> +#include <libcamera/base/span.h>
>  #include <libcamera/base/unique_fd.h>
>  
>  namespace libcamera {
> @@ -31,8 +31,8 @@ public:
>         ~Process();
>  
>         int start(const std::string &path,
> -                 const std::vector<std::string> &args = std::vector<std::string>(),
> -                 const std::vector<int> &fds = std::vector<int>());
> +                 Span<const std::string> args = {},
> +                 Span<const int> fds = {});
>  
>         ExitStatus exitStatus() const { return exitStatus_; }
>         int exitCode() const { return exitCode_; }
> @@ -44,7 +44,7 @@ public:
>  private:
>         LIBCAMERA_DISABLE_COPY_AND_MOVE(Process)
>  
> -       void closeAllFdsExcept(const std::vector<int> &fds);
> +       void closeAllFdsExcept(Span<const int> fds);
>         int isolate();
>         void died(int wstatus);
>  
> diff --git a/src/libcamera/ipc_pipe_unixsocket.cpp b/src/libcamera/ipc_pipe_unixsocket.cpp
> index 668ec73b9..7ee7cac79 100644
> --- a/src/libcamera/ipc_pipe_unixsocket.cpp
> +++ b/src/libcamera/ipc_pipe_unixsocket.cpp
> @@ -28,10 +28,6 @@ IPCPipeUnixSocket::IPCPipeUnixSocket(const char *ipaModulePath,
>                                      const char *ipaProxyWorkerPath)
>         : IPCPipe()
>  {
> -       std::vector<int> fds;
> -       std::vector<std::string> args;
> -       args.push_back(ipaModulePath);
> -
>         socket_ = std::make_unique<IPCUnixSocket>();
>         UniqueFD fd = socket_->create();
>         if (!fd.isValid()) {
> @@ -39,8 +35,9 @@ IPCPipeUnixSocket::IPCPipeUnixSocket(const char *ipaModulePath,
>                 return;
>         }
>         socket_->readyRead.connect(this, &IPCPipeUnixSocket::readyRead);
> -       args.push_back(std::to_string(fd.get()));
> -       fds.push_back(fd.get());
> +
> +       std::array args{ std::string(ipaModulePath), std::to_string(fd.get()) };
> +       std::array fds{ fd.get() };
>  
>         proc_ = std::make_unique<Process>();
>         int ret = proc_->start(ipaProxyWorkerPath, args, fds);
> diff --git a/src/libcamera/process.cpp b/src/libcamera/process.cpp
> index 62e63255b..5603dc903 100644
> --- a/src/libcamera/process.cpp
> +++ b/src/libcamera/process.cpp
> @@ -235,8 +235,8 @@ Process::~Process()
>   * or a negative error code otherwise
>   */
>  int Process::start(const std::string &path,
> -                  const std::vector<std::string> &args,
> -                  const std::vector<int> &fds)
> +                  Span<const std::string> args,
> +                  Span<const int> fds)
>  {
>         int ret;
>  
> @@ -282,9 +282,9 @@ int Process::start(const std::string &path,
>         }
>  }
>  
> -void Process::closeAllFdsExcept(const std::vector<int> &fds)
> +void Process::closeAllFdsExcept(Span<const int> fds)
>  {
> -       std::vector<int> v(fds);
> +       std::vector<int> v(fds.begin(), fds.end());
>         sort(v.begin(), v.end());
>  
>         ASSERT(v.empty() || v.front() >= 0);
> diff --git a/test/process/process_test.cpp b/test/process/process_test.cpp
> index e9f5e7e9b..a88d8fef1 100644
> --- a/test/process/process_test.cpp
> +++ b/test/process/process_test.cpp
> @@ -5,9 +5,9 @@
>   * Process test
>   */
>  
> +#include <array>
>  #include <iostream>
>  #include <unistd.h>
> -#include <vector>
>  
>  #include <libcamera/base/event_dispatcher.h>
>  #include <libcamera/base/thread.h>
> @@ -48,8 +48,7 @@ protected:
>                 Timer timeout;
>  
>                 int exitCode = 42;
> -               vector<std::string> args;
> -               args.push_back(to_string(exitCode));
> +               std::array args{ to_string(exitCode) };
>                 proc_.finished.connect(this, &ProcessTest::procFinished);
>  
>                 /* Test that kill() on an unstarted process is safe. */
> -- 
> 2.49.0
>
Laurent Pinchart March 26, 2025, 2:08 p.m. UTC | #2 
Hi Barnabás,

Thank you for the patch.

On Tue, Mar 25, 2025 at 07:08:18PM +0100, Barnabás Pőcze wrote:
> Use `libcamera::Span` whenever applicable.
> 
> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>

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

> ---
>  include/libcamera/internal/process.h  | 8 ++++----
>  src/libcamera/ipc_pipe_unixsocket.cpp | 9 +++------
>  src/libcamera/process.cpp             | 8 ++++----
>  test/process/process_test.cpp         | 5 ++---
>  4 files changed, 13 insertions(+), 17 deletions(-)
> 
> diff --git a/include/libcamera/internal/process.h b/include/libcamera/internal/process.h
> index 030a1e66e..e4f5bb979 100644
> --- a/include/libcamera/internal/process.h
> +++ b/include/libcamera/internal/process.h
> @@ -9,9 +9,9 @@
>  
>  #include <signal.h>
>  #include <string>
> -#include <vector>
>  
>  #include <libcamera/base/signal.h>
> +#include <libcamera/base/span.h>
>  #include <libcamera/base/unique_fd.h>
>  
>  namespace libcamera {
> @@ -31,8 +31,8 @@ public:
>  	~Process();
>  
>  	int start(const std::string &path,
> -		  const std::vector<std::string> &args = std::vector<std::string>(),
> -		  const std::vector<int> &fds = std::vector<int>());
> +		  Span<const std::string> args = {},
> +		  Span<const int> fds = {});
>  
>  	ExitStatus exitStatus() const { return exitStatus_; }
>  	int exitCode() const { return exitCode_; }
> @@ -44,7 +44,7 @@ public:
>  private:
>  	LIBCAMERA_DISABLE_COPY_AND_MOVE(Process)
>  
> -	void closeAllFdsExcept(const std::vector<int> &fds);
> +	void closeAllFdsExcept(Span<const int> fds);
>  	int isolate();
>  	void died(int wstatus);
>  
> diff --git a/src/libcamera/ipc_pipe_unixsocket.cpp b/src/libcamera/ipc_pipe_unixsocket.cpp
> index 668ec73b9..7ee7cac79 100644
> --- a/src/libcamera/ipc_pipe_unixsocket.cpp
> +++ b/src/libcamera/ipc_pipe_unixsocket.cpp
> @@ -28,10 +28,6 @@ IPCPipeUnixSocket::IPCPipeUnixSocket(const char *ipaModulePath,
>  				     const char *ipaProxyWorkerPath)
>  	: IPCPipe()
>  {
> -	std::vector<int> fds;
> -	std::vector<std::string> args;
> -	args.push_back(ipaModulePath);
> -
>  	socket_ = std::make_unique<IPCUnixSocket>();
>  	UniqueFD fd = socket_->create();
>  	if (!fd.isValid()) {
> @@ -39,8 +35,9 @@ IPCPipeUnixSocket::IPCPipeUnixSocket(const char *ipaModulePath,
>  		return;
>  	}
>  	socket_->readyRead.connect(this, &IPCPipeUnixSocket::readyRead);
> -	args.push_back(std::to_string(fd.get()));
> -	fds.push_back(fd.get());
> +
> +	std::array args{ std::string(ipaModulePath), std::to_string(fd.get()) };
> +	std::array fds{ fd.get() };
>  
>  	proc_ = std::make_unique<Process>();
>  	int ret = proc_->start(ipaProxyWorkerPath, args, fds);
> diff --git a/src/libcamera/process.cpp b/src/libcamera/process.cpp
> index 62e63255b..5603dc903 100644
> --- a/src/libcamera/process.cpp
> +++ b/src/libcamera/process.cpp
> @@ -235,8 +235,8 @@ Process::~Process()
>   * or a negative error code otherwise
>   */
>  int Process::start(const std::string &path,
> -		   const std::vector<std::string> &args,
> -		   const std::vector<int> &fds)
> +		   Span<const std::string> args,
> +		   Span<const int> fds)
>  {
>  	int ret;
>  
> @@ -282,9 +282,9 @@ int Process::start(const std::string &path,
>  	}
>  }
>  
> -void Process::closeAllFdsExcept(const std::vector<int> &fds)
> +void Process::closeAllFdsExcept(Span<const int> fds)
>  {
> -	std::vector<int> v(fds);
> +	std::vector<int> v(fds.begin(), fds.end());
>  	sort(v.begin(), v.end());
>  
>  	ASSERT(v.empty() || v.front() >= 0);
> diff --git a/test/process/process_test.cpp b/test/process/process_test.cpp
> index e9f5e7e9b..a88d8fef1 100644
> --- a/test/process/process_test.cpp
> +++ b/test/process/process_test.cpp
> @@ -5,9 +5,9 @@
>   * Process test
>   */
>  
> +#include <array>
>  #include <iostream>
>  #include <unistd.h>
> -#include <vector>
>  
>  #include <libcamera/base/event_dispatcher.h>
>  #include <libcamera/base/thread.h>
> @@ -48,8 +48,7 @@ protected:
>  		Timer timeout;
>  
>  		int exitCode = 42;
> -		vector<std::string> args;
> -		args.push_back(to_string(exitCode));
> +		std::array args{ to_string(exitCode) };
>  		proc_.finished.connect(this, &ProcessTest::procFinished);
>  
>  		/* Test that kill() on an unstarted process is safe. */

Patch
diff mbox series 

diff --git a/include/libcamera/internal/process.h b/include/libcamera/internal/process.h
index 030a1e66e..e4f5bb979 100644
--- a/include/libcamera/internal/process.h
+++ b/include/libcamera/internal/process.h
@@ -9,9 +9,9 @@ 
 
 #include <signal.h>
 #include <string>
-#include <vector>
 
 #include <libcamera/base/signal.h>
+#include <libcamera/base/span.h>
 #include <libcamera/base/unique_fd.h>
 
 namespace libcamera {
@@ -31,8 +31,8 @@  public:
 	~Process();
 
 	int start(const std::string &path,
-		  const std::vector<std::string> &args = std::vector<std::string>(),
-		  const std::vector<int> &fds = std::vector<int>());
+		  Span<const std::string> args = {},
+		  Span<const int> fds = {});
 
 	ExitStatus exitStatus() const { return exitStatus_; }
 	int exitCode() const { return exitCode_; }
@@ -44,7 +44,7 @@  public:
 private:
 	LIBCAMERA_DISABLE_COPY_AND_MOVE(Process)
 
-	void closeAllFdsExcept(const std::vector<int> &fds);
+	void closeAllFdsExcept(Span<const int> fds);
 	int isolate();
 	void died(int wstatus);
 
diff --git a/src/libcamera/ipc_pipe_unixsocket.cpp b/src/libcamera/ipc_pipe_unixsocket.cpp
index 668ec73b9..7ee7cac79 100644
--- a/src/libcamera/ipc_pipe_unixsocket.cpp
+++ b/src/libcamera/ipc_pipe_unixsocket.cpp
@@ -28,10 +28,6 @@  IPCPipeUnixSocket::IPCPipeUnixSocket(const char *ipaModulePath,
 				     const char *ipaProxyWorkerPath)
 	: IPCPipe()
 {
-	std::vector<int> fds;
-	std::vector<std::string> args;
-	args.push_back(ipaModulePath);
-
 	socket_ = std::make_unique<IPCUnixSocket>();
 	UniqueFD fd = socket_->create();
 	if (!fd.isValid()) {
@@ -39,8 +35,9 @@  IPCPipeUnixSocket::IPCPipeUnixSocket(const char *ipaModulePath,
 		return;
 	}
 	socket_->readyRead.connect(this, &IPCPipeUnixSocket::readyRead);
-	args.push_back(std::to_string(fd.get()));
-	fds.push_back(fd.get());
+
+	std::array args{ std::string(ipaModulePath), std::to_string(fd.get()) };
+	std::array fds{ fd.get() };
 
 	proc_ = std::make_unique<Process>();
 	int ret = proc_->start(ipaProxyWorkerPath, args, fds);
diff --git a/src/libcamera/process.cpp b/src/libcamera/process.cpp
index 62e63255b..5603dc903 100644
--- a/src/libcamera/process.cpp
+++ b/src/libcamera/process.cpp
@@ -235,8 +235,8 @@  Process::~Process()
  * or a negative error code otherwise
  */
 int Process::start(const std::string &path,
-		   const std::vector<std::string> &args,
-		   const std::vector<int> &fds)
+		   Span<const std::string> args,
+		   Span<const int> fds)
 {
 	int ret;
 
@@ -282,9 +282,9 @@  int Process::start(const std::string &path,
 	}
 }
 
-void Process::closeAllFdsExcept(const std::vector<int> &fds)
+void Process::closeAllFdsExcept(Span<const int> fds)
 {
-	std::vector<int> v(fds);
+	std::vector<int> v(fds.begin(), fds.end());
 	sort(v.begin(), v.end());
 
 	ASSERT(v.empty() || v.front() >= 0);
diff --git a/test/process/process_test.cpp b/test/process/process_test.cpp
index e9f5e7e9b..a88d8fef1 100644
--- a/test/process/process_test.cpp
+++ b/test/process/process_test.cpp
@@ -5,9 +5,9 @@ 
  * Process test
  */
 
+#include <array>
 #include <iostream>
 #include <unistd.h>
-#include <vector>
 
 #include <libcamera/base/event_dispatcher.h>
 #include <libcamera/base/thread.h>
@@ -48,8 +48,7 @@  protected:
 		Timer timeout;
 
 		int exitCode = 42;
-		vector<std::string> args;
-		args.push_back(to_string(exitCode));
+		std::array args{ to_string(exitCode) };
 		proc_.finished.connect(this, &ProcessTest::procFinished);
 
 		/* Test that kill() on an unstarted process is safe. */