Xen devel - Nov 2005 - Re: [Xen-changelog] Make it possible to run valgrind on code linked with the libxc libraries.

On Tue, Nov 22, 2005 at 06:20:06PM +0000, Xen patchbot -unstable
wrote:
> diff -r c1c8da6f2afe -r 602aefe7bd48 tools/libxc/xg_private.h
> --- a/tools/libxc/xg_private.h	Mon Nov 21 18:08:44 2005
> +++ b/tools/libxc/xg_private.h	Tue Nov 22 15:31:16 2005
> @@ -15,6 +15,16 @@
>  
>  #include <xen/linux/privcmd.h>
>  #include <xen/memory.h>
> +
> +/* valgrind cannot see when a hypercall has filled in some values.  For
this
> +   reason, we must zero the dom0_op_t instance before a call, if using
> +   valgrind.  */
> +#ifdef VALGRIND
> +#define DECLARE_DOM0_OP dom0_op_t op; memset(&op, 0, sizeof(op))
> +#else
> +#define DECLARE_DOM0_OP dom0_op_t op
> +#endif
This is pretty ugly IMHO. Also, if someone does

if (foo)
        bla;
else
        DECLARE_DOM0_OP;

it breaks.

Is the micro-optimization of not initializing the structures to 0
worth it? I really doubt it. But if yes, please at least do something
like

#ifdef VALGRIND
#define DECLARE_DOM0_OP(name) dom0_op_t name = {
        .cmd = 0,
	.interface_version = 0, 
	.pad = { 0 }
}
#else
#define #define DECLARE_DOM0_OP(name) dom0_op_t name;
#endif

Cheers,
Muli
-- 
Muli Ben-Yehuda
http://www.mulix.org | http://mulix.livejournal.com/


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel

On Tue, Nov 22, 2005 at 09:47:55PM +0200, Muli Ben-Yehuda wrote:
> On Tue, Nov 22, 2005 at 06:20:06PM +0000, Xen patchbot -unstable wrote:
> > diff -r c1c8da6f2afe -r 602aefe7bd48 tools/libxc/xg_private.h
> > --- a/tools/libxc/xg_private.h	Mon Nov 21 18:08:44 2005
> > +++ b/tools/libxc/xg_private.h	Tue Nov 22 15:31:16 2005
> > @@ -15,6 +15,16 @@
> >  
> >  #include <xen/linux/privcmd.h>
> >  #include <xen/memory.h>
> > +
> > +/* valgrind cannot see when a hypercall has filled in some values. 
For this
> > +   reason, we must zero the dom0_op_t instance before a call, if
using
> > +   valgrind.  */
> > +#ifdef VALGRIND
> > +#define DECLARE_DOM0_OP dom0_op_t op; memset(&op, 0, sizeof(op))
> > +#else
> > +#define DECLARE_DOM0_OP dom0_op_t op
> > +#endif
> 
> This is pretty ugly IMHO. Also, if someone does
> 
> if (foo)
>         bla;
> else
>         DECLARE_DOM0_OP;
> 
> it breaks.
There''s no argument that it''s ugly, but you''d have to
be pretty strange to be
trying to declare a variable inside a single statement else branch.
> Is the micro-optimization of not initializing the structures to 0
> worth it? I really doubt it.
I have no idea.  Perhaps someone could do some benchmarking?  I just tried to
make sure I didn''t add any overhead when all I was doing was supporting
a
debugging tool.  Personally, I would have no problem with the structures being
initialised unconditionally, and spending a few cycles, but other people would
disagree I''m sure.
> But if yes, please at least do something
> like
> 
> #ifdef VALGRIND
> #define DECLARE_DOM0_OP(name) dom0_op_t name = {
>         .cmd = 0,
> 	.interface_version = 0, 
> 	.pad = { 0 }
> }
> #else
> #define #define DECLARE_DOM0_OP(name) dom0_op_t name;
> #endif
Keir''s already changed it to dom0_op_t op = { 0 } which I think ought
to do
the trick well enough.

Cheers,

Ewan.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xensource.com
http://lists.xensource.com/xen-devel