On 11/13/2017 03:50 AM, Martin Kletzander wrote: > It is literally only a wrapper around virBitmapNewData() and > virBitmapFormat(), only the naming was wrong since it was introduced. in commit id '7d8afc47'. > And because we have virBitmap*String functions where the meaning of > the 'String' is constant, this might confuse someone. And some would say DataFormatWhat - cannot please everyone though ;-) because "naming is hard" (per abologna). FWIW: When you noted the 'String' is constant, I immediately thought about a "const char *String" meaning it wouldn't need to be free'd. > > Signed-off-by: Martin Kletzander <mkletzan@xxxxxxxxxx> > --- > src/libvirt_private.syms | 2 +- > src/util/virbitmap.c | 6 +++--- > src/util/virbitmap.h | 4 ++-- > tests/virbitmaptest.c | 4 ++-- > tools/virsh-domain.c | 4 ++-- > tools/virsh-host.c | 2 +- > 6 files changed, 11 insertions(+), 11 deletions(-) > So like patch 04/21 - patch 06/21 also hasn't shown up yet, so please consider the following for comments and R-b (all the others showed up). 1. The commit message needs massaging - "fromvirBitmapToString" to be "from virBitmapToString" 2. The 3rd argument should be on it's only line (like noted in 03) With the slight adjustment below, consider both 05 and 06 as: Reviewed-by: John Ferlan <jferlan@xxxxxxxxxx> John [...] > diff --git a/tests/virbitmaptest.c b/tests/virbitmaptest.c > index c24d4a72f70a..488796719dd9 100644 > --- a/tests/virbitmaptest.c > +++ b/tests/virbitmaptest.c There's a comment above here that needs adjustment ... s/DataToString/DataFormat to be consistent... > @@ -336,12 +336,12 @@ test5(const void *v ATTRIBUTE_UNUSED) > data2[4] != 0x04) > goto error; > > - if (!(str = virBitmapDataToString(data, sizeof(data)))) > + if (!(str = virBitmapDataFormat(data, sizeof(data)))) > goto error; > if (STRNEQ(str, "0,9,34")) > goto error; > VIR_FREE(str); > - if (!(str = virBitmapDataToString(data2, len2))) > + if (!(str = virBitmapDataFormat(data2, len2))) > goto error; > if (STRNEQ(str, "0,2,9,15,34")) > goto error; [...] -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
