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

Re: [libvirt] [RFC PATCH 6/6] tests: Use virBitmap for qemu command line caps



On Wed, Feb 09, 2011 at 10:33:01 -0700, Eric Blake wrote:
> On 02/09/2011 09:02 AM, Jiri Denemark wrote:
> > This needs to be squashed into the previous patch but is provided
> > separately for easier review.
> > ---
> >  src/qemu/qemu_capabilities.c |   14 +
> >  src/qemu/qemu_capabilities.h |    2 +
> >  tests/qemuhelptest.c         |  727 +++++++++++++++++++++---------------------
> >  tests/qemuxml2argvtest.c     |  468 ++++++++++++++--------------
> >  4 files changed, 617 insertions(+), 594 deletions(-)
> > 
> >  void
> > +qemuCapsSetList(virBitmapPtr caps, ...)
> > +{
> > +    va_list list;
> > +    int flag;
> > +
> > +    va_start(list, caps);
> > +    while ((flag = va_arg(list, int)) < QEMU_CAPS_LAST)

This is the bug I was talking about in the previous email. I should rather use
"enum qemuCapsFlags" instead of "int" in case someone passes -fshort-enum
option to gcc which would result in the enum being represented as char rather
than int.

> > +        ignore_value(virBitmapSetBit(caps, flag));
> > +    va_end(list);
> 
> QEMU_CAPS_LAST as a terminal seems a bit awkward.  Would it be any
> better to require 0 to be the terminal?

The problem is that 0 is a valid flag value (QEMU_CAPS_KQEMU) addressing the
lowest bit. We could reserve the value for a terminal but in that case we
could never make use of the lowest bit in the bitmap. Not that it would make a
huge difference but QEMU_CAPS_LAST just seemed good enough to me :-) Also it's
kinda nice to have a call like

    qemuCapsSetList(caps, QEMU_CAPS_1, QEMU_CAPS_2, ..., QEMU_CAPS_LAST)

Jirka


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