lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  PHC 
Open Source and information security mailing list archives
 
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Tue, 4 Apr 2017 14:58:08 +0200
From:   Hans Verkuil <hverkuil@...all.nl>
To:     Lee Jones <lee.jones@...aro.org>
Cc:     hans.verkuil@...co.com, mchehab@...nel.org,
        linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
        kernel@...inux.com, patrice.chotard@...com,
        linux-media@...r.kernel.org, benjamin.gaignard@...com
Subject: Re: [PATCH 1/2] [media] cec: Move capability check inside #if

On 04/04/2017 02:54 PM, Lee Jones wrote:
> On Tue, 04 Apr 2017, Hans Verkuil wrote:
> 
>> 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@...aro.org>
>>> ---
>>>  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.
> 
> I saw the #else.  It's inert code that becomes function-less.

No, it isn't. It clears the CAP_RC bit so it isn't returned in the CEC_ADAP_G_CAPS ioctl.
Drivers set this cap bit if they want RC support (they typically want it), but if the
config option isn't there then the capability should be removed.

Regards,

	Hans

> 
>> 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?
> 
> Sure.
> 

Powered by blists - more mailing lists