Re: [PATCH 5/8] v4l2-flash: Flash ops aren't mandatory

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Sakari,

On 06/14/2017 11:19 PM, Sakari Ailus wrote:
> Hi Jacek,
> 
> On Wed, Jun 14, 2017 at 11:14:13PM +0200, Jacek Anaszewski wrote:
>> Hi Sakari,
>>
>> On 06/14/2017 11:47 AM, Sakari Ailus wrote:
>>> None of the flash operations are not mandatory and therefore there should
>>> be no need for the flash ops structure either. Accept NULL.
>>>
>>> Signed-off-by: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx>
>>> ---
>>>  drivers/media/v4l2-core/v4l2-flash-led-class.c | 4 ++--
>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/media/v4l2-core/v4l2-flash-led-class.c b/drivers/media/v4l2-core/v4l2-flash-led-class.c
>>> index 6d69119..fdb79da 100644
>>> --- a/drivers/media/v4l2-core/v4l2-flash-led-class.c
>>> +++ b/drivers/media/v4l2-core/v4l2-flash-led-class.c
>>> @@ -18,7 +18,7 @@
>>>  #include <media/v4l2-flash-led-class.h>
>>>  
>>>  #define has_flash_op(v4l2_flash, op)				\
>>> -	(v4l2_flash && v4l2_flash->ops->op)
>>> +	(v4l2_flash && v4l2_flash->ops && v4l2_flash->ops->op)
>>
>> This change doesn't seem to be related to the patch subject.
> 
> Yes, it is: if there's a chance that ops is NULL, then you have to test here
> you actually have the ops struct around. The test is no longer in
> v4l2_flash_init().

Indeed.

Reviewed-by: Jacek Anaszewski <jacek.anaszewski@xxxxxxxxx>

>>>  #define call_flash_op(v4l2_flash, op, arg)			\
>>>  		(has_flash_op(v4l2_flash, op) ?			\
>>> @@ -618,7 +618,7 @@ struct v4l2_flash *v4l2_flash_init(
>>>  	struct v4l2_subdev *sd;
>>>  	int ret;
>>>  
>>> -	if (!fled_cdev || !ops || !config)
>>> +	if (!fled_cdev || !config)
>>>  		return ERR_PTR(-EINVAL);
>>>  
>>>  	led_cdev = &fled_cdev->led_cdev;
>>>
> 

-- 
Best regards,
Jacek Anaszewski



[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]

  Powered by Linux