Re: [PATCH 03/21] util: Make prefix optional in virBitampString

[Martin Kletzander <mkletzan@xxxxxxxxxx>]

On Mon, Nov 13, 2017 at 02:22:20PM -0500, John Ferlan wrote:
> $subj:
>
> s/virBitampString/virBitmapString/

hehe, good catch

> On 11/13/2017 03:50 AM, Martin Kletzander wrote:
> > Signed-off-by: Martin Kletzander <mkletzan@xxxxxxxxxx>
> > ---
> >  src/qemu/qemu_capabilities.c | 4 ++--
> >  src/util/virbitmap.c         | 6 ++++--
> >  src/util/virbitmap.h         | 2 +-
> >  3 files changed, 7 insertions(+), 5 deletions(-)
>
> FWIW: For some reason 4/21 hasn't yet made it to the mail server near
> me. I don't know why, it's just lost or on vacation. The only comment I
> have on it is that the commit message "util: Rename virBitmapString
> tovirBitmapToString" needs a slight adjustment "...String to virBit..."

You mean add a space between "to" and virBitmapToString?  It is there.

> I have one adjustment below regarding one arg per line in function...
> Consider this a:
>
> Reviewed-by: John Ferlan <jferlan@xxxxxxxxxx>
>
> For this and 04/21 because I'm way to lazy to figure out how to generate
> a reply to something I don't have the original email...

Sure, mentioning here is enough.

> John
>
> > diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
> > index 7cb091056b48..c87feefb3be3 100644
> > --- a/src/qemu/qemu_capabilities.c
> > +++ b/src/qemu/qemu_capabilities.c
> > @@ -1509,7 +1509,7 @@ int virQEMUCapsParseHelpStr(const char *qemu,
> >                                     qemuCaps, check_yajl) < 0)
> >          goto cleanup;
> >
> > -    strflags = virBitmapString(qemuCaps->flags);
> > +    strflags = virBitmapString(qemuCaps->flags, true);
> >      VIR_DEBUG("Version %u.%u.%u, cooked version %u, flags %s",
> >                major, minor, micro, *version, NULLSTR(strflags));
> >      VIR_FREE(strflags);
> > @@ -2376,7 +2376,7 @@ virQEMUCapsClear(virQEMUCapsPtr qemuCaps,
> >
> >  char *virQEMUCapsFlagsString(virQEMUCapsPtr qemuCaps)
> >  {
> > -    return virBitmapString(qemuCaps->flags);
> > +    return virBitmapString(qemuCaps->flags, true);
> >  }
> >
> >
> > diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c
> > index 57a8fbf82c78..cb6600074781 100644
> > --- a/src/util/virbitmap.c
> > +++ b/src/util/virbitmap.c
> > @@ -312,17 +312,19 @@ int virBitmapGetBit(virBitmapPtr bitmap, size_t b, bool *result)
> >  /**
> >   * virBitmapString:
> >   * @bitmap: Pointer to bitmap
> > + * @prefix: Whether to prepend "0x"
>
> Or "prehex" ;-)
>
> >   *
> >   * Convert @bitmap to printable string.
> >   *
> >   * Returns pointer to the string or NULL on error.
> >   */
> > -char *virBitmapString(virBitmapPtr bitmap)
> > +char *virBitmapString(virBitmapPtr bitmap, bool prefix)
>
> One line for each argument is the more "recent" style...

Sure, but a) mainly because otherwise it's a long line and b) it's along the
other functions that do the same so the reading style is the same, but here it
would just stand out as the only function, I think.  Anyway, I fixed that as
well.

> >  {
> >      virBuffer buf = VIR_BUFFER_INITIALIZER;
> >      size_t sz;
> >
> > -    virBufferAddLit(&buf, "0x");
> > +    if (prefix)
> > +        virBufferAddLit(&buf, "0x");
> >
> >      sz = bitmap->map_len;
> >
> > diff --git a/src/util/virbitmap.h b/src/util/virbitmap.h
> > index ffa3c42d792b..dc8fb71a07b8 100644
> > --- a/src/util/virbitmap.h
> > +++ b/src/util/virbitmap.h
> > @@ -80,7 +80,7 @@ bool virBitmapIsBitSet(virBitmapPtr bitmap, size_t b)
> >  int virBitmapGetBit(virBitmapPtr bitmap, size_t b, bool *result)
> >      ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(3) ATTRIBUTE_RETURN_CHECK;
> >
> > -char *virBitmapString(virBitmapPtr bitmap)
> > +char *virBitmapString(virBitmapPtr bitmap, bool prefix)
> >      ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK;
> >
> >  char *virBitmapFormat(virBitmapPtr bitmap);

Attachment: signature.asc
Description: Digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list