[libcamera-devel] [PATCH v3 11/17] libcamera: process: Manage pipe fds by UniqueFD

[Jacopo Mondi]

Hi Laurent,

On Mon, Nov 29, 2021 at 01:57:46AM +0200, Laurent Pinchart wrote:
> From: Hirokazu Honda <hiroh at chromium.org>
>
> Manages the file descriptors owned by Process for pipe by UniqueFDs.
>
> 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 96748a3676e4..95e67e105a92 100644
> --- a/include/libcamera/internal/process.h
> +++ b/include/libcamera/internal/process.h
> @@ -12,6 +12,7 @@
>  #include <vector>
>
>  #include <libcamera/base/signal.h>
> +#include <libcamera/base/unique_fd.h>
>
>  namespace libcamera {
>
> @@ -75,8 +76,9 @@ private:
>  	std::list<Process *> processes_;
>
>  	struct sigaction oldsa_;
> +

Unrelated but ok!

>  	EventNotifier *sigEvent_;
> -	int pipe_[2];
> +	UniqueFD pipe_[2];
>  };
>
>  } /* namespace libcamera */
> diff --git a/src/libcamera/process.cpp b/src/libcamera/process.cpp
> index eca1b30039b8..0e6b4e1dde58 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()
>  {
>  	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] = UniqueFD(pipe[0]);
> +	pipe_[1] = UniqueFD(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);
> +

UNrelated but ok

>  	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();

Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>

Thanks
  j

>  }
>
>  /**
> --
> Regards,
>
> Laurent Pinchart
>