[libcamera-devel] [PATCH 1/2] libcamera: process: fix compilation on Chromium OS

[Laurent Pinchart]

Hi Paul,

Thank you for the patch.

On Fri, Jul 12, 2019 at 05:29:08PM +0900, Paul Elder wrote:
> Commit 3d20beca6616 ("libcamera: Add Process and ProcessManager
> classes") causes the build to fail in the Chromium OS build environment,
> because the return values of some system calls are ignored. Fix this.

Same comments as for 2/2.

> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> ---
>  src/libcamera/process.cpp | 14 +++++++++++---
>  1 file changed, 11 insertions(+), 3 deletions(-)
> 
> diff --git a/src/libcamera/process.cpp b/src/libcamera/process.cpp
> index 9d8829d..eccf1dd 100644
> --- a/src/libcamera/process.cpp
> +++ b/src/libcamera/process.cpp
> @@ -69,7 +69,9 @@ namespace {
>  void sigact(int signal, siginfo_t *info, void *ucontext)
>  {
>  	char data = 0;
> -	write(ProcessManager::instance()->writePipe(), &data, sizeof(data));
> +	/* We're in a signal handler so we can't log any message,
> +	 * and we need to continue anyway. */

	/*
	 * ...
	 * ...
	 /

> +	(void)write(ProcessManager::instance()->writePipe(), &data, sizeof(data));
>  
>  	const struct sigaction &oldsa = ProcessManager::instance()->oldsa();
>  	if (oldsa.sa_flags & SA_SIGINFO) {
> @@ -85,7 +87,11 @@ void sigact(int signal, siginfo_t *info, void *ucontext)
>  void ProcessManager::sighandler(EventNotifier *notifier)
>  {
>  	char data;
> -	read(pipe_[0], &data, sizeof(data));
> +	if (read(pipe_[0], &data, sizeof(data))) {
> +		LOG(Process, Error)
> +			<< "failed to read byte from direct signal handler";

s/failed/Failed/
s/from direct signal handler/from signal handler pipe/

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

> +		return;
> +	}
>  
>  	for (auto it = processes_.begin(); it != processes_.end(); ) {
>  		Process *process = *it;
> @@ -128,7 +134,9 @@ ProcessManager::ProcessManager()
>  
>  	sigaction(SIGCHLD, &sa, NULL);
>  
> -	pipe2(pipe_, O_CLOEXEC | O_DIRECT | O_NONBLOCK);
> +	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);
>  	sigEvent_->activated.connect(this, &ProcessManager::sighandler);
>  }

-- 
Regards,

Laurent Pinchart