Re: [Xen-devel] [PATCH v2] libxl: Fix incorrect return of OSEVENT_HOOK macro

[Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>]

Daniel De Graaf writes ("[PATCH v2] libxl: Fix incorrect return of OSEVENT_HOOK 
macro"):
> Agreed, using the statement expression syntax seems cleaner here.
...
> +#define OSEVENT_HOOK(hookname,...) ({                                       \
> +    int rc;                                                                 \
> +    if (CTX->osevent_hooks) {                                               \
> +        CTX->osevent_in_hook++;                                             \
> +        rc = CTX->osevent_hooks->hookname(CTX->osevent_user, __VA_ARGS__);  \
> +        CTX->osevent_in_hook--;                                             \
> +    } else {                                                                \
> +        rc = 0;                                                             \
> +    }                                                                       \
> +    rc;                                                                     \
> +})

Thanks.  However, you need to use a different variable name to "rc".
"rc" might theoretically be present int __VA_ARGS__ in which case this
will go wrong.  Also if we ever enable -Wshadow, this construction
will cause a warning.  I suggest   int osevent_hook_rc;

You could preserve the unification of the two macros by having

 #define OSEVENT_HOOK(hookname,...)                                           \
     OSEVENT_HOOK_INTERN(osevent_hook_rc =, /*empty*/, hookname, __VA_ARGS__)

 #define OSEVENT_HOOK_VOID(hookname,...)                                      \
     OSEVENT_HOOK_INTERN(/*empty*/,         (void),    hookname, __VA_ARGS__)

or some such.  I don't know whether you like that idea; if you do
please go ahead and do it.  If not then I'm happy to take the patch
which duplicates the macro.

Ian.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel