[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 2/2] Add some DTrace/SystemTAP probes to the RPC code



On 09/30/2011 07:54 AM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange"<berrange redhat com>


Not much of a commit comment. Even mentioning that half of the patch is code motion rather than new content might be useful.

+
+/* Systemtap 1.2 headers have a bug where they cannot handle a
+ * variable declared with array type.  Work around this by casting all
+ * arguments.  This is some gross use of the preprocessor because
+ * PROBE is a var-arg macro, but it is better than the alternative of
+ * making all callers to PROBE have to be aware of the issues.  And
+ * hopefully, if we ever add a call to PROBE with other than 2 or 3

s/other than 2 or 3/more than 7/

+ * end arguments, you can figure out the pattern to extend this hack.
+ */
+#  define VIR_COUNT_ARGS(...) VIR_ARG9(__VA_ARGS__, 8, 7, 6, 5, 4, 3, 2, 1)
+#  define VIR_ARG9(_1, _2, _3, _4, _5, _6, _7, _8, _9, ...) _9
+#  define VIR_ADD_CAST_EXPAND(a, b, ...) VIR_ADD_CAST_PASTE(a, b, __VA_ARGS__)
+#  define VIR_ADD_CAST_PASTE(a, b, ...) a##b(__VA_ARGS__)
+
+/* The double cast is necessary to silence gcc warnings; any pointer
+ * can safely go to intptr_t and back to void *, which collapses
+ * arrays into pointers; while any integer can be widened to intptr_t
+ * then cast to void *.  */
+#  define VIR_ADD_CAST(a) ((void *)(intptr_t)(a))

Now that you've expanded the list, it looks awkward not having VIR_ADD_CAST1(a) VIR_ADD_CAST(a)

+#  define VIR_ADD_CAST2(a, b)                           \
+    VIR_ADD_CAST(a), VIR_ADD_CAST(b)
+#  define VIR_ADD_CAST3(a, b, c)                        \
+    VIR_ADD_CAST(a), VIR_ADD_CAST(b), VIR_ADD_CAST(c)
+#  define VIR_ADD_CAST4(a, b, c, d)                       \
+    VIR_ADD_CAST(a), VIR_ADD_CAST(b), VIR_ADD_CAST(c),    \
+    VIR_ADD_CAST(d)
+#  define VIR_ADD_CAST5(a, b, c, d, e)                    \
+    VIR_ADD_CAST(a), VIR_ADD_CAST(b), VIR_ADD_CAST(c),    \
+    VIR_ADD_CAST(d), VIR_ADD_CAST(e)
+#  define VIR_ADD_CAST6(a, b, c, d, e, f)                 \
+    VIR_ADD_CAST(a), VIR_ADD_CAST(b), VIR_ADD_CAST(c),    \
+    VIR_ADD_CAST(d), VIR_ADD_CAST(e), VIR_ADD_CAST(f)
+#  define VIR_ADD_CAST7(a, b, c, d, e, f, g)              \
+    VIR_ADD_CAST(a), VIR_ADD_CAST(b), VIR_ADD_CAST(c), \
+    VIR_ADD_CAST(d), VIR_ADD_CAST(e), VIR_ADD_CAST(f),      \
+    VIR_ADD_CAST(g)

Up to here is reachable...

+#  define VIR_ADD_CAST8(a, b, c, d, e, f, g, h)           \
+    VIR_ADD_CAST(a), VIR_ADD_CAST(b), VIR_ADD_CAST(c),    \
+    VIR_ADD_CAST(d), VIR_ADD_CAST(e), VIR_ADD_CAST(f),      \
+    VIR_ADD_CAST(g), VIR_ADD_CAST(h)
+#  define VIR_ADD_CAST9(a, b, c, d, e, f, g, h, i)        \
+    VIR_ADD_CAST(a), VIR_ADD_CAST(b), VIR_ADD_CAST(c),          \
+    VIR_ADD_CAST(d), VIR_ADD_CAST(e), VIR_ADD_CAST(f),      \
+    VIR_ADD_CAST(g), VIR_ADD_CAST(h), VIR_ADD_CAST(i)

but these two are impossible given your definition of VIR_COUNT_ARGS. To really support 9 arguments, VIR_COUNT_ARGS needs to call VIR_ARG11(__VA_ARGS__, 10, 9, ...).

ACK with that fixed. Everything else looks like a lot of stap black magic, but the end result is indeed a setup that can be easily traced to track traffic.

--
Eric Blake   eblake redhat com    +1-801-349-2682
Libvirt virtualization library http://libvirt.org


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]