Re: [PATCH v2 4/6] tools/libxc: Introduce ARRAY_SIZE() and replace handrolled examples

[Ian Campbell <Ian.Campbell@citrix.com>]

On Mon, 2014-06-09 at 16:41 +0100, Andrew Cooper wrote:
> The conditional test is needed because xc_private.h is included into minios
> for stubdomains, where ARRAY_SIZE is already defined.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> CC: Ian Campbell <Ian.Campbell@citrix.com>
> CC: Ian Jackson <Ian.Jackson@eu.citrix.com>
> 
> --
> v2: Find more areas of the codebase which are poking at libxc private internals
> ---
>  tools/libxc/xc_dom_arm.c |    2 +-
>  tools/libxc/xc_dom_x86.c |    4 ++--
>  tools/libxc/xc_minios.c  |    2 +-
>  tools/libxc/xc_private.h |    4 ++++
>  tools/misc/xen-hptool.c  |    2 --
>  tools/misc/xen-mfndump.c |    2 --

Not tools/misc/xenpm.c too?

tools/misc/* really didn't ought to be including xc_private.h I think.

Rather than make this worse how about defining XC_ARRAY_SIZE in the
public API? Or if not perhaps
   #undef ARRAY_SIZE /* We shouldn't be including xc_private.h */
in those so that an eventual removal of xc_private from them isn't
blocked? (I think I slightly prefer the second option)

minios really shouldn't be polluting the namespace like that either.
Perhaps that's a yack Ian J is going to cover as part of the rump kernel
stuff...

>  6 files changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/tools/libxc/xc_dom_arm.c b/tools/libxc/xc_dom_arm.c
> index 75f8363..cc64363 100644
> --- a/tools/libxc/xc_dom_arm.c
> +++ b/tools/libxc/xc_dom_arm.c
> @@ -230,7 +230,7 @@ static int set_mode(xc_interface *xch, domid_t domid, char *guest_type)
>  
>      domctl.domain = domid;
>      domctl.cmd    = XEN_DOMCTL_set_address_size;
> -    for ( i = 0; i < sizeof(types)/sizeof(types[0]); i++ )
> +    for ( i = 0; i < ARRAY_SIZE(types); i++ )
>          if ( !strcmp(types[i].guest, guest_type) )
>              domctl.u.address_size.size = types[i].size;
>      if ( domctl.u.address_size.size == 0 )
> diff --git a/tools/libxc/xc_dom_x86.c b/tools/libxc/xc_dom_x86.c
> index e034d62..bf06fe4 100644
> --- a/tools/libxc/xc_dom_x86.c
> +++ b/tools/libxc/xc_dom_x86.c
> @@ -716,7 +716,7 @@ static int x86_compat(xc_interface *xch, domid_t domid, char *guest_type)
>      memset(&domctl, 0, sizeof(domctl));
>      domctl.domain = domid;
>      domctl.cmd    = XEN_DOMCTL_set_address_size;
> -    for ( i = 0; i < sizeof(types)/sizeof(types[0]); i++ )
> +    for ( i = 0; i < ARRAY_SIZE(types); i++ )
>          if ( !strcmp(types[i].guest, guest_type) )
>              domctl.u.address_size.size = types[i].size;
>      if ( domctl.u.address_size.size == 0 )
> @@ -887,7 +887,7 @@ int arch_setup_bootlate(struct xc_dom_image *dom)
>      xen_pfn_t shinfo;
>      int i, rc;
>  
> -    for ( i = 0; i < sizeof(types) / sizeof(types[0]); i++ )
> +    for ( i = 0; i < ARRAY_SIZE(types); i++ )
>          if ( !strcmp(types[i].guest, dom->guest_type) )
>              pgd_type = types[i].pgd_type;
>  
> diff --git a/tools/libxc/xc_minios.c b/tools/libxc/xc_minios.c
> index e621417..e703684 100644
> --- a/tools/libxc/xc_minios.c
> +++ b/tools/libxc/xc_minios.c
> @@ -87,7 +87,7 @@ static int minios_privcmd_hypercall(xc_interface *xch, xc_osdep_handle h, privcm
>      int i, ret;
>  
>      call.op = hypercall->op;
> -    for (i = 0; i < sizeof(hypercall->arg) / sizeof(*hypercall->arg); i++)
> +    for (i = 0; i < ARRAY_SIZE(hypercall->arg); i++)
>  	call.args[i] = hypercall->arg[i];
>  
>      ret = HYPERVISOR_multicall(&call, 1);
> diff --git a/tools/libxc/xc_private.h b/tools/libxc/xc_private.h
> index 12476ec..213f607 100644
> --- a/tools/libxc/xc_private.h
> +++ b/tools/libxc/xc_private.h
> @@ -54,6 +54,10 @@
>  #define XC_BUILD_BUG_ON(p) ((void)sizeof(struct { int:-!!(p); }))
>  #endif
>  
> +#ifndef ARRAY_SIZE
> +#define ARRAY_SIZE(a) (sizeof(a) / sizeof(a[0]))
> +#endif
> +
>  /*
>  ** Define max dirty page cache to permit during save/restore -- need to balance 
>  ** keeping cache usage down with CPU impact of invalidating too often.
> diff --git a/tools/misc/xen-hptool.c b/tools/misc/xen-hptool.c
> index d0e8e90..a16213a 100644
> --- a/tools/misc/xen-hptool.c
> +++ b/tools/misc/xen-hptool.c
> @@ -4,8 +4,6 @@
>  #include <xenstore.h>
>  #include <unistd.h>
>  
> -#define ARRAY_SIZE(a) (sizeof (a) / sizeof ((a)[0]))
> -
>  static xc_interface *xch;
>  
>  void show_help(void)
> diff --git a/tools/misc/xen-mfndump.c b/tools/misc/xen-mfndump.c
> index 1af3bd8..33e4b91 100644
> --- a/tools/misc/xen-mfndump.c
> +++ b/tools/misc/xen-mfndump.c
> @@ -6,8 +6,6 @@
>  
>  #include "xg_save_restore.h"
>  
> -#define ARRAY_SIZE(a) (sizeof (a) / sizeof ((a)[0]))
> -
>  static xc_interface *xch;
>  
>  int help_func(int argc, char *argv[])