Re: [Xen-devel] [PATCH v6 1/7] xen: xsm: flask: introduce XENMAPSPACE_gmfn_share for memory sharing

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Fri, 3 Aug 2018, Jan Beulich wrote:
> >>> On 02.08.18 at 22:57, <sstabellini@xxxxxxxxxx> wrote:
> > On Thu, 2 Aug 2018, Jan Beulich wrote:
> >> >> > +/*
> >> >> > + * This action also requires that @current targets @d, but it has 
> >> >> > already been
> >> >> > + * checked somewhere higher in the call stack.
> >> >> 
> >> >> I'm not convinced it is a good idea to have such a dependency, even
> >> >> more so with this cloudy a reference. If there's another XSM check
> >> >> that has necessarily been done before, you should at least name it
> >> >> here so it's easy to later verify that the assumption still holds. But
> >> >> even better would imo be to re-do the check here, just in case.
> >> > 
> >> > I am fine with that. It should be just a matter of doing the following,
> >> > right?
> >> > 
> >> > static XSM_INLINE int xsm_map_gmfn_share(XSM_DEFAULT_ARG struct domain 
> >> > *d,
> >> >                                          struct domain *t)
> >> > {
> >> >     XSM_ASSERT_ACTION(XSM_TARGET);
> >> >     return xsm_default_action(XSM_TARGET, current->domain, d) &&
> >> >            xsm_default_action(action, current->domain, t);
> >> 
> >> Not exactly - xsm_default_action() doesn't return boolean, nor should
> >> the function here.
> > 
> > You are right, zero is success. So something like the following:
> > 
> >     int rc = xsm_default_action(XSM_TARGET, current->domain, d);
> >     if ( rc )
> >         return rc;
> >     return xsm_default_action(action, current->domain, t);
> 
> Didn't you inherit the patch from someone? I think we had already
> arrived at
> 
>     return xsm_default_action(XSM_TARGET, current->domain, d)
>             ?: xsm_default_action(action, current->domain, t);

If you are referring to the code style, I don't have an opinion, I am
happy to do the above. I changed it in these code snippets only because
in the other function (flask_map_gmfn_share) the ?: model would become
awkward as there are three consecutive checks. But here it would be
fine.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel