[libcamera-devel] [PATCH] utils: ipc: ipa_proxy_worker: Log IPCUnixSocket::send() failures

[Kieran Bingham]

On 17/08/2021 04:16, Laurent Pinchart wrote:
> The IPCUnixSocket::send() function may fail, in which case it can be
> useful for debugging to log an error message that tells which event was
> affected. Do so.
> 
> Reported-by: Coverity CID=354836
> Reported-by: Coverity CID=354837
> Reported-by: Coverity CID=354838
> Reported-by: Coverity CID=354839
> Reported-by: Coverity CID=354840
> Reported-by: Coverity CID=354841
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
> This stems from a Coverity report that complains about the lock of error
> checking for the return value of send(). As the send() function itself
> logs a message on failure, I'm not entirely sure if this is needed, but
> we do so when replying to synchronous methods, and there's no reason to
> have a different behaviour for events. We should either add a message
> here, or drop it from method replies.

I think coverity considers how an API is used, so indeed - here it
likely noticed "Hey you check this in other places, should you check it
here too"?

I think adding the print here is ok ... as it will be clearer that the
failure occurred from the specific proxy worker, so it adds value. The
::send() will report the strerror() as well, so overall this helps.

> Kieran, is the Reported-by tag processed by any script ? Can I write

There is no scripting no. It's just a reference.

> Reported-by: Coverity CID=354836-354841

If you wish ;-)

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

> 
> or something similar ?
> 
> ---
>  .../libcamera_templates/module_ipa_proxy_worker.cpp.tmpl     | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/utils/ipc/generators/libcamera_templates/module_ipa_proxy_worker.cpp.tmpl b/utils/ipc/generators/libcamera_templates/module_ipa_proxy_worker.cpp.tmpl
> index 164a4dd64882..8a88bd467da7 100644
> --- a/utils/ipc/generators/libcamera_templates/module_ipa_proxy_worker.cpp.tmpl
> +++ b/utils/ipc/generators/libcamera_templates/module_ipa_proxy_worker.cpp.tmpl
> @@ -164,7 +164,10 @@ private:
>  
>  		{{proxy_funcs.serialize_call(method|method_param_inputs, "_message.data()", "_message.fds()")}}
>  
> -		socket_.send(_message.payload());
> +		int _ret = socket_.send(_message.payload());
> +		if (_ret < 0)
> +			LOG({{proxy_worker_name}}, Error)
> +				<< "Sending event {{method.mojom_name}}() failed: " << _ret;
>  
>  		LOG({{proxy_worker_name}}, Debug) << "{{method.mojom_name}} done";
>  	}
>