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