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