Re: [Xen-devel] [PATCH v6 5/5] xen: introduce XENMEM_pin

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Mon, 2013-09-30 at 13:56 +0100, Stefano Stabellini wrote:
> On Mon, 30 Sep 2013, Jan Beulich wrote:
> > > +
> > > +    if ( !is_hardware_domain(d) )
> > > +    {
> > > +        rc = -EPERM;
> > > +        goto fail;
> > > +    }
> > > +
> > > +    memflags |= MEMF_bits(domain_clamp_alloc_bitsize(
> > > +        d,
> > > +        XENMEMF_get_address_bits(pin.in.mem_flags) ? :
> > > +        (BITS_PER_LONG+PAGE_SHIFT)));
> > > +
> > > +    for ( ; i < pin.in.nr_extents; i++ )
> > > +    {
> > > +        if ( unlikely(__copy_from_guest_offset(
> > > +                      &gpfn, pin.in.extent_start, i, 1)) )
> > > +        {
> > > +            rc = -EFAULT;
> > > +            goto fail;
> > > +        }
> > > +
> > > +        if ( generic_fls64(gpfn << PAGE_SHIFT) > memflags )
> > 
> > Didn't you mean MEMF_bits(memflags) here?
> 
> memflags is set to MEMF_bits(XXX) in the assignment above

memflags is quite a confusing name for such a variable and/or implicitly
relying on this internal knowledge of the flags layout later is overly
subtle.

If you don't want to use the accessor as Jan suggested perhaps membits
or just bits would be a better variable name, omitting the MEMF_bits
"wrapper" until the actual callsite which needs it or keep memflags
separately.

Ian.


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