On Mon, Jun 14, 2010 at 11:52:49AM +1000, Lawrence Stewart wrote: > On 06/13/10 20:10, Pawel Jakub Dawidek wrote: > >On Sun, Jun 13, 2010 at 02:39:55AM +0000, Lawrence Stewart wrote: > [snip] > >> > >>Modified: head/sys/sys/pcpu.h > >>============================================================================== > >>--- head/sys/sys/pcpu.h Sun Jun 13 01:27:29 2010 (r209118) > >>+++ head/sys/sys/pcpu.h Sun Jun 13 02:39:55 2010 (r209119) > >>@@ -106,6 +106,17 @@ extern uintptr_t dpcpu_off[]; > >> #define DPCPU_ID_GET(i, n) (*DPCPU_ID_PTR(i, n)) > >> #define DPCPU_ID_SET(i, n, v) (*DPCPU_ID_PTR(i, n) = v) > >> > >>+/* > >>+ * Utility macros. > >>+ */ > >>+#define DPCPU_SUM(n, var, sum) \ > >>+do { \ > >>+ (sum) = 0; \ > >>+ u_int i; \ > >>+ CPU_FOREACH(i) \ > >>+ (sum) += (DPCPU_ID_PTR(i, n))->var; \ > >>+} while (0) > > > >I'd suggest first swapping variable declaration and '(sum) = 0;'. > >Also using 'i' as a counter in macro can easly lead to name collision. > >If you need to do it, I'd suggest '_i' or something. > > Given that the DPCPU variable name space is flat and variable names have > to be unique, perhaps something like the following would address the > concerns raised? > > #define DPCPU_SUM(n, var, sum) \ > do { \ > u_int _##n##_i; \ > (sum) = 0; \ > CPU_FOREACH(_##n##_i) \ > (sum) += (DPCPU_ID_PTR(_##n##_i, n))->var; \ > } while (0)
You do not have to jump through this. Mostly by convention, in our kernel sources, names with "_" prefix are reserved for the infrastructure (cannot say implementation). I think it is quite safe to use _i for the iteration variable. As an example of this, look at sys/sys/mount.h, implementation of VFS_NEEDGIANT, VFS_LOCK_GIANT etc macros. They do use gcc ({}) extension to provide function-like macros, but this is irrelevant. Or, look at the VFS_ASSERT_GIANT that is exactly like what you need.
pgpEYRXSiENYx.pgp
Description: PGP signature