On Thu, 2010-08-12 at 22:16 +0200, Guennadi Liakhovetski wrote:
> "sd" and "err" are too common names to be used in macros for local variables.
> Prefix them with an underscore to avoid name clashing.
>
> Signed-off-by: Guennadi Liakhovetski <g.liakhovetski@xxxxxx>
I've reviewed these but not tetsed them. The functionally look OK.
I share Lawrence's concern about "__err" being considered a reserved
library symbol somewhere in the kernel sometime in the future, and still
having a good probability for name clashing
However
"git grep __err" reveals that "__err" is used in a few places as a local
variable with block scope in a macro: the same manner as it is used in
this patch. I didn't find a global declaration of "__err" in the
kernel.
"git grep '[^_a-zA-Z0-9]_err[^_a-z:]' | grep -v goto" also shows that
"_err" is popular in macros, but also with no apparent global
declaration in the kernel.
"git grep '[^_a-zA-Z0-9]_err_[^a-z]' | grep -v goto" shows that "_err_"
is not used anywhere in the kernel.
Reviewed-by: Andy Walls <awalls@xxxxxxxxxxxxxxxx>
Regards,
Andy
> ---
>
> v2:
>
> as suggested by Mauro, also patched v4l2_device_call_all and
> v4l2_device_call_until_err, as well as ivtv and cx18 specific macros
>
> drivers/media/video/cx18/cx18-driver.h | 19 +++++++---
> drivers/media/video/ivtv/ivtv-driver.h | 14 ++++++--
> include/media/v4l2-device.h | 57 ++++++++++++++++++++++----------
> 3 files changed, 63 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/media/video/cx18/cx18-driver.h b/drivers/media/video/cx18/cx18-driver.h
> index 9bc51a9..7e43c7b 100644
> --- a/drivers/media/video/cx18/cx18-driver.h
> +++ b/drivers/media/video/cx18/cx18-driver.h
> @@ -674,18 +674,25 @@ static inline int cx18_raw_vbi(const struct cx18 *cx)
>
> /* Call the specified callback for all subdevs with a grp_id bit matching the
> * mask in hw (if 0, then match them all). Ignore any errors. */
> -#define cx18_call_hw(cx, hw, o, f, args...) \
> - __v4l2_device_call_subdevs(&(cx)->v4l2_dev, \
> - !(hw) || (sd->grp_id & (hw)), o, f , ##args)
> +#define cx18_call_hw(cx, hw, o, f, args...) \
> + do { \
> + struct v4l2_subdev *__sd; \
> + __v4l2_device_call_subdevs_p(&(cx)->v4l2_dev, __sd, \
> + !(hw) || (__sd->grp_id & (hw)), o, f , ##args); \
> + } while (0)
>
> #define cx18_call_all(cx, o, f, args...) cx18_call_hw(cx, 0, o, f , ##args)
>
> /* Call the specified callback for all subdevs with a grp_id bit matching the
> * mask in hw (if 0, then match them all). If the callback returns an error
> * other than 0 or -ENOIOCTLCMD, then return with that error code. */
> -#define cx18_call_hw_err(cx, hw, o, f, args...) \
> - __v4l2_device_call_subdevs_until_err( \
> - &(cx)->v4l2_dev, !(hw) || (sd->grp_id & (hw)), o, f , ##args)
> +#define cx18_call_hw_err(cx, hw, o, f, args...) \
> +({ \
> + struct v4l2_subdev *__sd; \
> + __v4l2_device_call_subdevs_until_err_p(&(cx)->v4l2_dev, \
> + __sd, !(hw) || (__sd->grp_id & (hw)), o, f, \
> + ##args); \
> +})
>
> #define cx18_call_all_err(cx, o, f, args...) \
> cx18_call_hw_err(cx, 0, o, f , ##args)
> diff --git a/drivers/media/video/ivtv/ivtv-driver.h b/drivers/media/video/ivtv/ivtv-driver.h
> index 7580314..e61252c 100644
> --- a/drivers/media/video/ivtv/ivtv-driver.h
> +++ b/drivers/media/video/ivtv/ivtv-driver.h
> @@ -811,15 +811,23 @@ static inline int ivtv_raw_vbi(const struct ivtv *itv)
> /* Call the specified callback for all subdevs matching hw (if 0, then
> match them all). Ignore any errors. */
> #define ivtv_call_hw(itv, hw, o, f, args...) \
> - __v4l2_device_call_subdevs(&(itv)->v4l2_dev, !(hw) || (sd->grp_id & (hw)), o, f , ##args)
> + do { \
> + struct v4l2_subdev *__sd; \
> + __v4l2_device_call_subdevs_p(&(itv)->v4l2_dev, __sd, \
> + !(hw) || (__sd->grp_id & (hw)), o, f , ##args); \
> + } while (0)
>
> #define ivtv_call_all(itv, o, f, args...) ivtv_call_hw(itv, 0, o, f , ##args)
>
> /* Call the specified callback for all subdevs matching hw (if 0, then
> match them all). If the callback returns an error other than 0 or
> -ENOIOCTLCMD, then return with that error code. */
> -#define ivtv_call_hw_err(itv, hw, o, f, args...) \
> - __v4l2_device_call_subdevs_until_err(&(itv)->v4l2_dev, !(hw) || (sd->grp_id & (hw)), o, f , ##args)
> +#define ivtv_call_hw_err(itv, hw, o, f, args...) \
> +({ \
> + struct v4l2_subdev *__sd; \
> + __v4l2_device_call_subdevs_until_err_p(&(itv)->v4l2_dev, __sd, \
> + !(hw) || (__sd->grp_id & (hw)), o, f , ##args); \
> +})
>
> #define ivtv_call_all_err(itv, o, f, args...) ivtv_call_hw_err(itv, 0, o, f , ##args)
>
> diff --git a/include/media/v4l2-device.h b/include/media/v4l2-device.h
> index 8bcbd7a..fe10464 100644
> --- a/include/media/v4l2-device.h
> +++ b/include/media/v4l2-device.h
> @@ -101,46 +101,67 @@ void v4l2_device_unregister_subdev(struct v4l2_subdev *sd);
> /* Call the specified callback for all subdevs matching the condition.
> Ignore any errors. Note that you cannot add or delete a subdev
> while walking the subdevs list. */
> -#define __v4l2_device_call_subdevs(v4l2_dev, cond, o, f, args...) \
> +#define __v4l2_device_call_subdevs_p(v4l2_dev, sd, cond, o, f, args...) \
> do { \
> - struct v4l2_subdev *sd; \
> + list_for_each_entry((sd), &(v4l2_dev)->subdevs, list) \
> + if ((cond) && (sd)->ops->o && (sd)->ops->o->f) \
> + (sd)->ops->o->f((sd) , ##args); \
> + } while (0)
> +
> +#define __v4l2_device_call_subdevs(v4l2_dev, cond, o, f, args...) \
> + do { \
> + struct v4l2_subdev *__sd; \
> \
> - list_for_each_entry(sd, &(v4l2_dev)->subdevs, list) \
> - if ((cond) && sd->ops->o && sd->ops->o->f) \
> - sd->ops->o->f(sd , ##args); \
> + __v4l2_device_call_subdevs_p(v4l2_dev, __sd, cond, o, \
> + f , ##args); \
> } while (0)
>
> /* Call the specified callback for all subdevs matching the condition.
> If the callback returns an error other than 0 or -ENOIOCTLCMD, then
> return with that error code. Note that you cannot add or delete a
> subdev while walking the subdevs list. */
> -#define __v4l2_device_call_subdevs_until_err(v4l2_dev, cond, o, f, args...) \
> +#define __v4l2_device_call_subdevs_until_err_p(v4l2_dev, sd, cond, o, f, args...) \
> ({ \
> - struct v4l2_subdev *sd; \
> - long err = 0; \
> + long __err = 0; \
> \
> - list_for_each_entry(sd, &(v4l2_dev)->subdevs, list) { \
> - if ((cond) && sd->ops->o && sd->ops->o->f) \
> - err = sd->ops->o->f(sd , ##args); \
> - if (err && err != -ENOIOCTLCMD) \
> + list_for_each_entry((sd), &(v4l2_dev)->subdevs, list) { \
> + if ((cond) && (sd)->ops->o && (sd)->ops->o->f) \
> + __err = (sd)->ops->o->f((sd) , ##args); \
> + if (__err && __err != -ENOIOCTLCMD) \
> break; \
> } \
> - (err == -ENOIOCTLCMD) ? 0 : err; \
> + (__err == -ENOIOCTLCMD) ? 0 : __err; \
> +})
> +
> +#define __v4l2_device_call_subdevs_until_err(v4l2_dev, cond, o, f, args...) \
> +({ \
> + struct v4l2_subdev *__sd; \
> + __v4l2_device_call_subdevs_until_err_p(v4l2_dev, __sd, cond, o, \
> + f, args...); \
> })
>
> /* Call the specified callback for all subdevs matching grp_id (if 0, then
> match them all). Ignore any errors. Note that you cannot add or delete
> a subdev while walking the subdevs list. */
> -#define v4l2_device_call_all(v4l2_dev, grpid, o, f, args...) \
> - __v4l2_device_call_subdevs(v4l2_dev, \
> - !(grpid) || sd->grp_id == (grpid), o, f , ##args)
> +#define v4l2_device_call_all(v4l2_dev, grpid, o, f, args...) \
> + do { \
> + struct v4l2_subdev *__sd; \
> + \
> + __v4l2_device_call_subdevs_p(v4l2_dev, __sd, \
> + !(grpid) || __sd->grp_id == (grpid), o, f , \
> + ##args); \
> + } while (0)
>
> /* Call the specified callback for all subdevs matching grp_id (if 0, then
> match them all). If the callback returns an error other than 0 or
> -ENOIOCTLCMD, then return with that error code. Note that you cannot
> add or delete a subdev while walking the subdevs list. */
> #define v4l2_device_call_until_err(v4l2_dev, grpid, o, f, args...) \
> - __v4l2_device_call_subdevs_until_err(v4l2_dev, \
> - !(grpid) || sd->grp_id == (grpid), o, f , ##args)
> +({ \
> + struct v4l2_subdev *__sd; \
> + __v4l2_device_call_subdevs_until_err_p(v4l2_dev, __sd, \
> + !(grpid) || __sd->grp_id == (grpid), o, f , \
> + ##args); \
> +})
>
> #endif
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
