[libcamera-devel] [RFC PATCH 04/10] libcamera: Process: Manage pipe fds by ScopedFD
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Sun Jun 6 20:07:20 CEST 2021
Hi Hiro,
Thank you for the patch.
On Thu, Apr 15, 2021 at 05:38:37PM +0900, Hirokazu Honda wrote:
> Process owns the file descriptors for pipe. They should be
> managed by ScopedFDs to avoid the leakage.
>
> Signed-off-by: Hirokazu Honda <hiroh at chromium.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
> include/libcamera/internal/process.h | 4 +++-
> src/libcamera/process.cpp | 16 ++++++++++------
> 2 files changed, 13 insertions(+), 7 deletions(-)
>
> diff --git a/include/libcamera/internal/process.h b/include/libcamera/internal/process.h
> index 254cda85..b3bb24df 100644
> --- a/include/libcamera/internal/process.h
> +++ b/include/libcamera/internal/process.h
> @@ -11,6 +11,7 @@
> #include <string>
> #include <vector>
>
> +#include <libcamera/scoped_file_descriptor.h>
> #include <libcamera/signal.h>
>
> namespace libcamera {
> @@ -75,8 +76,9 @@ private:
> std::list<Process *> processes_;
>
> struct sigaction oldsa_;
> +
> EventNotifier *sigEvent_;
> - int pipe_[2];
> + ScopedFD pipe_[2];
> };
>
> } /* namespace libcamera */
> diff --git a/src/libcamera/process.cpp b/src/libcamera/process.cpp
> index 40a434a6..bb51d73b 100644
> --- a/src/libcamera/process.cpp
> +++ b/src/libcamera/process.cpp
> @@ -69,7 +69,7 @@ void sigact(int signal, siginfo_t *info, void *ucontext)
> void ProcessManager::sighandler([[maybe_unused]] EventNotifier *notifier)
> {
> char data;
> - ssize_t ret = read(pipe_[0], &data, sizeof(data));
> + ssize_t ret = read(pipe_[0].get(), &data, sizeof(data));
> if (ret < 0) {
> LOG(Process, Error)
> << "Failed to read byte from signal handler pipe";
> @@ -129,10 +129,15 @@ ProcessManager::ProcessManager()
>
> sigaction(SIGCHLD, &sa, NULL);
>
> - if (pipe2(pipe_, O_CLOEXEC | O_DIRECT | O_NONBLOCK))
> + int pipe[2];
> + if (pipe2(pipe, O_CLOEXEC | O_DIRECT | O_NONBLOCK))
> LOG(Process, Fatal)
> << "Failed to initialize pipe for signal handling";
> - sigEvent_ = new EventNotifier(pipe_[0], EventNotifier::Read);
> +
> + pipe_[0] = ScopedFD(pipe[0]);
> + pipe_[1] = ScopedFD(pipe[1]);
> +
> + sigEvent_ = new EventNotifier(pipe_[0].get(), EventNotifier::Read);
> sigEvent_->activated.connect(this, &ProcessManager::sighandler);
>
> self_ = this;
> @@ -141,9 +146,8 @@ ProcessManager::ProcessManager()
> ProcessManager::~ProcessManager()
> {
> sigaction(SIGCHLD, &oldsa_, NULL);
> +
> delete sigEvent_;
> - close(pipe_[0]);
> - close(pipe_[1]);
>
> self_ = nullptr;
> }
> @@ -170,7 +174,7 @@ ProcessManager *ProcessManager::instance()
> */
> int ProcessManager::writePipe() const
> {
> - return pipe_[1];
> + return pipe_[1].get();
> }
>
> /**
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list