On Sun, 13 Jun 2010, 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:
Log:
Add a utility macro to simplify calculating an aggregate sum from a DPCPU
   counter variable.

   Sponsored by:        FreeBSD Foundation
   Reviewed by: jhb, rpaulo, rwatson (previous version of patch)
   MFC after:   1 week

Modified:
   head/sys/sys/pcpu.h

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;'.

Of course.

Also fix the lexical style bugs:
- space instead of tab after #define
- "do {" at the beginning of a line (see queue.h for normal style)
- missing blank line after declarations
- missing braces around loop body (*).


Can do (will wait until consensus on other issues is reached first before tweaking though).

Also using 'i' as a counter in macro can easly lead to name collision.

I had a similar concern but after chatting with John on IRC felt it wasn't such a big deal in this case.

If you need to do it, I'd suggest '_i' or something.

Could do... is it worth it?

Maybe it would be better to make it an inline function rather than macro?

Inlining it could be annoying with respect to the types used in the function prototype, no? I suspect it would be more useful keeping it as a macro if possible.

Inlining it is impossible for the same reason that parenthesizing all the
macro args would be impossible -- `var' is a name and parenthesizing its
use would give the syntax error foo->(var).  Not sure about (n).  It's
a little surprising that parenthesizing works for lvalues like (sum)
(it works because lvalues include certain expressions including
parenthesized ones, but I wonder if it ever makes a difference).

BTW, one reason I liked BSD code more than gnu code is that it didn't
use so many macros.  Macros should only exist when they are not just
syntactic sugar, like DPCPU_SUM() and unlike CPU_FOREACH().

(*) style(9) recommends leaving out these unnecessary braces.  However,
mistakes like CPU_FOREACH() look less like syntax errors with the braces
than without them; the braces at least confuse indent(1) into not making
a mess.  indent(1) already knows that it doesn't understand macros so it
doesn't reformat them, so this doesn't quite apply here.

Bruce
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Reply via email to