Re: [Xen-devel] [PATCH v6 02/16] Rename PSR sysctl/domctl interfaces and xsm policy to make them be general

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 08.10.17 at 09:23, <yi.y.sun@xxxxxxxxxxxxxxx> wrote:
> --- a/xen/arch/x86/domctl.c
> +++ b/xen/arch/x86/domctl.c
> @@ -1438,67 +1438,66 @@ long arch_do_domctl(
>          }
>          break;
>  
> -    case XEN_DOMCTL_psr_cat_op:
> -        switch ( domctl->u.psr_cat_op.cmd )
> +    case XEN_DOMCTL_psr_alloc:
> +#define domctl_psr_get_val(d, domctl, type, copyback) ({   \
> +    uint32_t v;                                            \
> +    int r = psr_get_val(d, domctl->u.psr_alloc.target,     \

As indicated in reply to Roger's request to drop the double
underscores you previously had here, using entirely
"conventional" names isn't good either, as this risks conflicts
with variables declared in outer scopes. A single trailing
underscore would be advised here, or some other mechanism
to obviously disambiguate them.

Furthermore if already you pass in variables (which is fine
but not strictly necessary for a macro with such a restricted
scope), uses like domctl above need to have parentheses
added.

>          default:
>              ret = -EOPNOTSUPP;
>              break;
>          }
> +
> +#undef domctl_psr_get_val
> +
>          break;

Looking especially at this part I think the #define and #undef
aren't placed well. Such temporary helper macros should not
have wider scope than necessary - you really only need them
from the first GET case label unil immediately before default:.
At the very least I'd like to ask for the #define to be moved
inside the switch() statement, and the #undef ahead of
default:.

With these taken care of
Acked-by: Jan Beulich <jbeulich@xxxxxxxx>

Jan


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