Re: [PATCH 1/2] [media] cec: Move capability check inside #if

From: Hans Verkuil
Date: Tue Apr 04 2017 - 08:39:17 EST


On 04/04/2017 02:32 PM, Lee Jones wrote:
> If CONFIG_RC_CORE is not enabled then none of the RC code will be
> executed anyway, so we're placing the capability check inside the
>
> Signed-off-by: Lee Jones <lee.jones@xxxxxxxxxx>
> ---
> drivers/media/cec/cec-core.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/media/cec/cec-core.c b/drivers/media/cec/cec-core.c
> index 37217e2..06a312c 100644
> --- a/drivers/media/cec/cec-core.c
> +++ b/drivers/media/cec/cec-core.c
> @@ -234,10 +234,10 @@ struct cec_adapter *cec_allocate_adapter(const struct cec_adap_ops *ops,
> return ERR_PTR(res);
> }
>
> +#if IS_REACHABLE(CONFIG_RC_CORE)
> if (!(caps & CEC_CAP_RC))
> return adap;
>
> -#if IS_REACHABLE(CONFIG_RC_CORE)
> /* Prepare the RC input device */
> adap->rc = rc_allocate_device(RC_DRIVER_SCANCODE);
> if (!adap->rc) {
>

Not true, there is an #else further down.

That said, this code is clearly a bit confusing.

It would be better if at the beginning of the function we'd have this:

#if !IS_REACHABLE(CONFIG_RC_CORE)
caps &= ~CEC_CAP_RC;
#endif

and then drop the #else bit and (as you do in this patch) move the #if up.

Can you make a new patch for this?

Thanks!

Hans