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@redhat.com> --- 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@redhat.com> 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; [...]