Re: [PATCH] media: cec: extron-da-hd-4k-plus: add return check for wait_for_completion*()

[Hans Verkuil]

On 10/11/2024 13:58, Karol Przybylski wrote:
> According to scheduler/completion.rst, return status of wait_for_completion*()
> function variants should be checked or be accompanied by explanation.
> 
> I examined code in extron-da-hd-4k-plus.c and it does look like the return value
> should be checked, but perhaps there is a reason for ignoring it.

There is no need to check the return code in this specific case.

If the wait was successful, then port->edid_blocks will be set, so
effectively that 'if' is the result check of the wait_for_completion
call.

Regards,

	Hans

> I drafted a patch for this, but I'm not entirely sure how to approach error handling in this case.
> 
> Discovered in coverity, CID1599679
> 
> Signed-off-by: Karol Przybylski <karprzy7@xxxxxxxxx>
> ---
>  .../cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c    | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c b/drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c
> index cfbfc4c1b2e6..83a790117411 100644
> --- a/drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c
> +++ b/drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c
> @@ -559,9 +559,12 @@ static void extron_read_edid(struct extron_port *port)
>  
>  	extron->edid_reading = true;
>  
> -	if (!extron_send_and_wait(extron, port, cmd, reply))
> -		wait_for_completion_killable_timeout(&extron->edid_completion,
> +	if (!extron_send_and_wait(extron, port, cmd, reply)) {
> +		ret = wait_for_completion_killable_timeout(&extron->edid_completion,
>  						     msecs_to_jiffies(1000));
> +		if (ret < 0)
> +			goto unlock;
> +	}
>  	if (port->edid_blocks) {
>  		extron_parse_edid(port);
>  		port->read_edid = true;