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

Re: [PATCH v8 04/12] xen: add basic hypervisor filesystem support

On 14.05.20 13:58, Jan Beulich wrote:
On 14.05.2020 11:50, Jürgen Groß wrote:
On 14.05.20 09:59, Jan Beulich wrote:
On 08.05.2020 17:34, Juergen Gross wrote:
+#define HYPFS_FIXEDSIZE_INIT(var, typ, nam, contvar, wr) \
+    struct hypfs_entry_leaf __read_mostly var = {        \
+        .e.type = typ,                                   \
+        .e.encoding = XEN_HYPFS_ENC_PLAIN,               \
+        .e.name = nam,                                   \
+        .e.size = sizeof(contvar),                       \
+        .e.max_size = wr ? sizeof(contvar) : 0,          \
+        .e.read = hypfs_read_leaf,                       \
+        .e.write = wr,                                   \
+        .content = &contvar,                             \
+    }

At the example of this, some of the macros look like they want
parentheses added around uses of some of their parameters.

Hmm, which ones? As I've understood from previous patch reviews by you
you only want those parameters with parantheses where they are really

- var is a plain variable, so no parantheses
- typ _should_ be a XEN_HYPFS_TYPE_* define, so probably no parantheses
   (its usage in the macro doesn't call for using parantheses anyway)
- nam might be a candidate, but I can't come up with a reason to put it
   in parantheses here
- contvar has to be a variable (otherwise sizeof(contvar) wouldn't
   work), so no parantheses
- wr is a function pointer or NULL, so no parantheses

You have a point for uses as initializers, as there's no lower
precedence operator than the assignment ones except comma,
which would need parenthesizing in the macro invocation already.
However, I disagree on what you say about contvar and wr -
contvar is expected, but not required to be just an identifier.
And wr in turn is expected but not required to by an identifier
or NULL. I.e. the respective two lines where I think parentheses
can't be avoided are

         .e.max_size = (wr) ? sizeof(contvar) : 0,


         .content = &(contvar),





Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.