Message ID | 20250325180821.1456154-7-barnabas.pocze@ideasonboard.com |
---|---|
State | New |
Headers | show |
Series |
|
Related | show |
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 >
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. */
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. */
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(-)