| Message ID | e1a835c0f4c43a61bdee2950e1752051c92ca73d.1513682135.git.mchehab@s-opensource.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Mauro, Thanks for the patch. Please see my comments below. On Tue, Dec 19, 2017 at 09:18:22AM -0200, Mauro Carvalho Chehab wrote: > The __V4L2_SUBDEV_MK_GET_TRY() macro is used to define > 3 functions that have the same arguments. The code of those > functions is simple enough to just declare them, de-obfuscating > the code. > > While here, replace BUG_ON() by WARN_ON() as there's no reason > why to panic the Kernel if this fails. > > Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com> > --- > include/media/v4l2-subdev.h | 40 ++++++++++++++++++++++++++++------------ > 1 file changed, 28 insertions(+), 12 deletions(-) > > diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h > index 71b8ff4b2e0e..443e5e019006 100644 > --- a/include/media/v4l2-subdev.h > +++ b/include/media/v4l2-subdev.h > @@ -896,19 +896,35 @@ struct v4l2_subdev_fh { > container_of(fh, struct v4l2_subdev_fh, vfh) > > #if defined(CONFIG_VIDEO_V4L2_SUBDEV_API) > -#define __V4L2_SUBDEV_MK_GET_TRY(rtype, fun_name, field_name) \ > - static inline struct rtype * \ > - fun_name(struct v4l2_subdev *sd, \ > - struct v4l2_subdev_pad_config *cfg, \ > - unsigned int pad) \ > - { \ > - BUG_ON(pad >= sd->entity.num_pads); \ > - return &cfg[pad].field_name; \ > - } > +static inline struct v4l2_mbus_framefmt > +*v4l2_subdev_get_try_format(struct v4l2_subdev *sd, > + struct v4l2_subdev_pad_config *cfg, > + unsigned int pad) > +{ > + if (WARN_ON(pad >= sd->entity.num_pads)) > + pad = 0; > + return &cfg[pad].try_fmt; After I suggested this I came to think what happens if there are no pads? How about adding, before the first check: if (WARN_ON(!sd->entity.num_pads)) return NULL; Instead of copying the code, you could still use a macro while having the function declaration itself separate from the macro. Up to you. > +} > > -__V4L2_SUBDEV_MK_GET_TRY(v4l2_mbus_framefmt, v4l2_subdev_get_try_format, try_fmt) > -__V4L2_SUBDEV_MK_GET_TRY(v4l2_rect, v4l2_subdev_get_try_crop, try_crop) > -__V4L2_SUBDEV_MK_GET_TRY(v4l2_rect, v4l2_subdev_get_try_compose, try_compose) > +static inline struct v4l2_rect > +*v4l2_subdev_get_try_crop(struct v4l2_subdev *sd, > + struct v4l2_subdev_pad_config *cfg, > + unsigned int pad) > +{ > + if (WARN_ON(pad >= sd->entity.num_pads)) > + pad = 0; > + return &cfg[pad].try_crop; > +} > + > +static inline struct v4l2_rect > +*v4l2_subdev_get_try_compose(struct v4l2_subdev *sd, > + struct v4l2_subdev_pad_config *cfg, > + unsigned int pad) > +{ > + if (WARN_ON(pad >= sd->entity.num_pads)) > + pad = 0; > + return &cfg[pad].try_compose; > +} > #endif > > extern const struct v4l2_file_operations v4l2_subdev_fops; > -- > 2.14.3 >
diff --git a/include/media/v4l2-subdev.h b/include/media/v4l2-subdev.h index 71b8ff4b2e0e..443e5e019006 100644 --- a/include/media/v4l2-subdev.h +++ b/include/media/v4l2-subdev.h @@ -896,19 +896,35 @@ struct v4l2_subdev_fh { container_of(fh, struct v4l2_subdev_fh, vfh) #if defined(CONFIG_VIDEO_V4L2_SUBDEV_API) -#define __V4L2_SUBDEV_MK_GET_TRY(rtype, fun_name, field_name) \ - static inline struct rtype * \ - fun_name(struct v4l2_subdev *sd, \ - struct v4l2_subdev_pad_config *cfg, \ - unsigned int pad) \ - { \ - BUG_ON(pad >= sd->entity.num_pads); \ - return &cfg[pad].field_name; \ - } +static inline struct v4l2_mbus_framefmt +*v4l2_subdev_get_try_format(struct v4l2_subdev *sd, + struct v4l2_subdev_pad_config *cfg, + unsigned int pad) +{ + if (WARN_ON(pad >= sd->entity.num_pads)) + pad = 0; + return &cfg[pad].try_fmt; +} -__V4L2_SUBDEV_MK_GET_TRY(v4l2_mbus_framefmt, v4l2_subdev_get_try_format, try_fmt) -__V4L2_SUBDEV_MK_GET_TRY(v4l2_rect, v4l2_subdev_get_try_crop, try_crop) -__V4L2_SUBDEV_MK_GET_TRY(v4l2_rect, v4l2_subdev_get_try_compose, try_compose) +static inline struct v4l2_rect +*v4l2_subdev_get_try_crop(struct v4l2_subdev *sd, + struct v4l2_subdev_pad_config *cfg, + unsigned int pad) +{ + if (WARN_ON(pad >= sd->entity.num_pads)) + pad = 0; + return &cfg[pad].try_crop; +} + +static inline struct v4l2_rect +*v4l2_subdev_get_try_compose(struct v4l2_subdev *sd, + struct v4l2_subdev_pad_config *cfg, + unsigned int pad) +{ + if (WARN_ON(pad >= sd->entity.num_pads)) + pad = 0; + return &cfg[pad].try_compose; +} #endif extern const struct v4l2_file_operations v4l2_subdev_fops;
The __V4L2_SUBDEV_MK_GET_TRY() macro is used to define 3 functions that have the same arguments. The code of those functions is simple enough to just declare them, de-obfuscating the code. While here, replace BUG_ON() by WARN_ON() as there's no reason why to panic the Kernel if this fails. Signed-off-by: Mauro Carvalho Chehab <mchehab@s-opensource.com> --- include/media/v4l2-subdev.h | 40 ++++++++++++++++++++++++++++------------ 1 file changed, 28 insertions(+), 12 deletions(-)