Re: [PATCH v2] V4L2: avoid name conflicts in macros
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.liakhovet...@gmx.de
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 awa...@md.metrocast.net
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...) \
+({
Re: [PATCH v2] V4L2: avoid name conflicts in macros
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.
Identifiers that begin with a double underscore (as you have used) are
reserved for standard headers and defining them invokes undefined
behaviour.
From the C99 standard para 7.1.3
http://www.open-std.org/jtc1/sc22/wg14/www/docs/n1124.pdf :
All identifiers that begin with an underscore and either an uppercase
letter or another underscore are always reserved for any use.
...
No other identifiers are reserved. If the program declares or defines
an identifier in a context in which it is reserved (other than as
allowed by 7.1.4), or defines a reserved identifier as a macro name, the
behavior is undefined.
It is preferable to use a single underscore and a lowercase prefix such
as the library or header name e.g. _v4l_sd, _v4l_err, etc.
Aside - the copious use of macros encourages leakage of identifiers and
often breaks encapsulation as here, where the implementation details of
subdevs are exported. A range of inline accessor/mutator functions
would be preferable.
-- Lawrence Rust
Signed-off-by: Guennadi Liakhovetski g.liakhovet...@gmx.de
---
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...) \
-
[PATCH v2] V4L2: avoid name conflicts in macros
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.liakhovet...@gmx.de
---
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...)
\
