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.ai...@linux.intel.com>
>>> ---
>>>  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.anaszew...@gmail.com>

>>>  #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

Reply via email to