[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



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@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.