Re: [Xen-devel] [PATCH v3 22/28] tools/libxc: Modify bitmap operations to take void pointers

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Tue, Mar 15, 2016 at 03:35:18PM +0000, Andrew Cooper wrote:
> The type of the pointer to a bitmap is not interesting; it does not affect the
> representation of the block of bits being pointed to.
> 
> Make the libxc functions consistent with those in Xen, so they can work just
> as well with 'unsigned int *' based bitmaps.
> 
> As part of doing so, change the implementation to be in terms of char rather
> than unsigned long.  This fixes alignment concerns with ARM.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> ---
> CC: Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>
> CC: Wei Liu <wei.liu2@xxxxxxxxxx>
> 
> v3:
>  * Implement in terms of char rather than unsigned long to fix alignment
>    issues for ARM.

I'm very illiterate on this subject so I've CC Julien and Stefano for
input.

> v2:
>  * New
> ---
>  tools/libxc/xc_bitops.h | 37 ++++++++++++++++++++-----------------
>  1 file changed, 20 insertions(+), 17 deletions(-)
> 
> diff --git a/tools/libxc/xc_bitops.h b/tools/libxc/xc_bitops.h
> index cd749f4..c2c96d3 100644
> --- a/tools/libxc/xc_bitops.h
> +++ b/tools/libxc/xc_bitops.h
> @@ -6,70 +6,73 @@
>  #include <stdlib.h>
>  #include <string.h>
>  
> +/* Needed by several includees, but no longer used for bitops. */
>  #define BITS_PER_LONG (sizeof(unsigned long) * 8)
>  #define ORDER_LONG (sizeof(unsigned long) == 4 ? 5 : 6)
>  
> -#define BITMAP_ENTRY(_nr,_bmap) ((_bmap))[(_nr)/BITS_PER_LONG]
> -#define BITMAP_SHIFT(_nr) ((_nr) % BITS_PER_LONG)
> +#define BITMAP_ENTRY(_nr,_bmap) ((_bmap))[(_nr) / 8]
> +#define BITMAP_SHIFT(_nr) ((_nr) % 8)
>  
>  /* calculate required space for number of longs needed to hold nr_bits */
>  static inline int bitmap_size(int nr_bits)
>  {
> -    int nr_long, nr_bytes;
> -    nr_long = (nr_bits + BITS_PER_LONG - 1) >> ORDER_LONG;
> -    nr_bytes = nr_long * sizeof(unsigned long);
> -    return nr_bytes;
> +    return (nr_bits + 7) >> 8;
>  }
>  
> -static inline unsigned long *bitmap_alloc(int nr_bits)
> +static inline void *bitmap_alloc(int nr_bits)
>  {
>      return calloc(1, bitmap_size(nr_bits));
>  }
>  
> -static inline void bitmap_set(unsigned long *addr, int nr_bits)
> +static inline void bitmap_set(void *addr, int nr_bits)
>  {
>      memset(addr, 0xff, bitmap_size(nr_bits));
>  }
>  
> -static inline void bitmap_clear(unsigned long *addr, int nr_bits)
> +static inline void bitmap_clear(void *addr, int nr_bits)
>  {
>      memset(addr, 0, bitmap_size(nr_bits));
>  }
>  
> -static inline int test_bit(int nr, unsigned long *addr)
> +static inline int test_bit(int nr, const void *_addr)
>  {
> +    const char *addr = _addr;
>      return (BITMAP_ENTRY(nr, addr) >> BITMAP_SHIFT(nr)) & 1;
>  }
>  
> -static inline void clear_bit(int nr, unsigned long *addr)
> +static inline void clear_bit(int nr, void *_addr)
>  {
> +    char *addr = _addr;
>      BITMAP_ENTRY(nr, addr) &= ~(1UL << BITMAP_SHIFT(nr));
>  }
>  
> -static inline void set_bit(int nr, unsigned long *addr)
> +static inline void set_bit(int nr, void *_addr)
>  {
> +    char *addr = _addr;
>      BITMAP_ENTRY(nr, addr) |= (1UL << BITMAP_SHIFT(nr));
>  }
>  
> -static inline int test_and_clear_bit(int nr, unsigned long *addr)
> +static inline int test_and_clear_bit(int nr, void *addr)
>  {
>      int oldbit = test_bit(nr, addr);
>      clear_bit(nr, addr);
>      return oldbit;
>  }
>  
> -static inline int test_and_set_bit(int nr, unsigned long *addr)
> +static inline int test_and_set_bit(int nr, void *addr)
>  {
>      int oldbit = test_bit(nr, addr);
>      set_bit(nr, addr);
>      return oldbit;
>  }
>  
> -static inline void bitmap_or(unsigned long *dst, const unsigned long *other,
> +static inline void bitmap_or(void *_dst, const void *_other,
>                               int nr_bits)
>  {
> -    int i, nr_longs = (bitmap_size(nr_bits) / sizeof(unsigned long));
> -    for ( i = 0; i < nr_longs; ++i )
> +    char *dst = _dst;
> +    const char *other = _other;
> +    int i;
> +    for ( i = 0; i < bitmap_size(nr_bits); ++i )
>          dst[i] |= other[i];
>  }
>  
> -- 
> 2.1.4
> 

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