Re: [Xen-devel] [PATCH] libxc: zero-initialize structures in macros

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 02/09/16 17:39, Tamas K Lengyel wrote:
> While debugging applications built on top of libxc with Valgrind we get a lot
> of complaining about relying on uninitialized values allocated in libxc.
> While these warnings are safe to ignore, zero-initializing the structures
> reduces Valgrind clutter a lot and aids in spotting real bugs.
>
> Signed-off-by: Tamas K Lengyel <tamas.lengyel@xxxxxxxxxxxx>
> ---
> Cc: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>
> Cc: Wei Liu <wei.liu2@xxxxxxxxxx>
> ---
>  tools/libxc/xc_private.h | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/tools/libxc/xc_private.h b/tools/libxc/xc_private.h
> index 75b761c..4e9073b 100644
> --- a/tools/libxc/xc_private.h
> +++ b/tools/libxc/xc_private.h
> @@ -59,11 +59,11 @@ struct iovec {
>  #include <sys/uio.h>
>  #endif
>  
> -#define DECLARE_DOMCTL struct xen_domctl domctl
> -#define DECLARE_SYSCTL struct xen_sysctl sysctl
> -#define DECLARE_PHYSDEV_OP struct physdev_op physdev_op
> -#define DECLARE_FLASK_OP struct xen_flask_op op
> -#define DECLARE_PLATFORM_OP struct xen_platform_op platform_op
> +#define DECLARE_DOMCTL struct xen_domctl domctl = {0}
> +#define DECLARE_SYSCTL struct xen_sysctl sysctl = {0}
> +#define DECLARE_PHYSDEV_OP struct physdev_op physdev_op = {0}
> +#define DECLARE_FLASK_OP struct xen_flask_op op = {0}
> +#define DECLARE_PLATFORM_OP struct xen_platform_op platform_op = {0}

I specifically took those out in the past, because it hides real
problems from Valgrind.

Instead, I would recommend removing these wrappers entirely.  They serve
no useful purpose.

Taking a random example of xc_get_pfn_type_batch(), it would be rather
more efficient to write

...
    DECLARE_HYPERCALL_BOUNCE(arr, sizeof(*arr) * num,
XC_HYPERCALL_BUFFER_BOUNCE_BOTH);
    struct xen_domctl domctl = {
        .cmd = XEN_DOMCTL_getpageframeinfo3,
        .domain = dom,
        .u.getpageframeinfo3.num = num,
    };
...

as it permits the compiler more freedom in how xen_domctl gets
constructed, as well as being able to plainly see exactly what is done
to the memory.

~Andrew

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