Re: [REVIEW PATCH for v3.15 1/4] v4l2-subdev.h: fix sparse error with v4l2_subdev_notify

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Hans,

On Monday 17 March 2014 12:45:16 Hans Verkuil wrote:
> On 03/17/2014 12:44 PM, Laurent Pinchart wrote:
> > On Saturday 15 March 2014 14:08:00 Hans Verkuil wrote:
> >> From: Hans Verkuil <hans.verkuil@cisco.com>
> >> 
> >> The notify function is a void function, yet the v4l2_subdev_notify
> >> define uses it in a ? : construction, which causes sparse warnings.
> >> 
> >> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> >> ---
> >> 
> >>  include/media/v4l2-subdev.h | 8 +++++---
> >>  1 file changed, 5 insertions(+), 3 deletions(-)
> >> 
> >> diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h
> >> index 28f4d8c..0fbf669 100644
> >> --- a/include/media/v4l2-subdev.h
> >> +++ b/include/media/v4l2-subdev.h
> >> @@ -692,9 +692,11 @@ void v4l2_subdev_init(struct v4l2_subdev *sd,
> >> 
> >>  		(sd)->ops->o->f((sd) , ##args) : -ENOIOCTLCMD))
> >>  
> >>  /* Send a notification to v4l2_device. */
> >> 
> >> -#define v4l2_subdev_notify(sd, notification, arg)			   \
> >> -	((!(sd) || !(sd)->v4l2_dev || !(sd)->v4l2_dev->notify) ? -ENODEV : \
> >> -	 (sd)->v4l2_dev->notify((sd), (notification), (arg)))
> >> +#define v4l2_subdev_notify(sd, notification, arg)				\
> >> +	do {									\
> >> +		if ((sd) && (sd)->v4l2_dev && (sd)->v4l2_dev->notify)		\
> >> +			(sd)->v4l2_dev->notify((sd), (notification), (arg));	\
> >> +	} while (0)
> > 
> > The construct would prevent using v4l2_subdev_notify() as an expression.
> > What about turning the macro into an inline function instead ?
> 
> How can you use a void function in an expression anyway? That was the whole
> point of the sparse error.

In a for loop for instance ?

	for (i = 0; i < n; v4l2_subdev_notify(), ++i)

I agree it's a bit far-fetched, but as we need to modify the macro, I'd take 
that as an opportunity to turn it into an inline function.

> >>  #define v4l2_subdev_has_op(sd, o, f) \
> >>  
> >>  	((sd)->ops->o && (sd)->ops->o->f)

-- 
Regards,

Laurent Pinchart