[libvirt] [PATCH 07/21] util: Introduce virBitmapNewString

[John Ferlan]

On 11/13/2017 03:50 AM, Martin Kletzander wrote:
> Our bitmaps can be represented as data (raw bytes for which we have
> virBitmapNewData() and virBitmapToData()), human representation (list
> of numbers in a string for which we have virBitmapParse() and
> virBitmapFormat()) and hexadecimal string (for which we have only
> virBitmapToString()).  So let's add the missing complement for the
> last one so that we can parse hexadecimal strings.
> 
> Signed-off-by: Martin Kletzander <mkletzan at redhat.com>
> ---
>  src/libvirt_private.syms |  1 +
>  src/util/virbitmap.c     | 36 ++++++++++++++++++++++++++++++++++++
>  src/util/virbitmap.h     |  4 ++++
>  tests/virbitmaptest.c    | 38 ++++++++++++++++++++++++++++++++++++++
>  4 files changed, 79 insertions(+)
> 

With a couple of minor/noted adjustments below,

Reviewed-by: John Ferlan <jferlan at redhat.com>

John

> diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
> index e535167e28fc..57df411602b9 100644
> --- a/src/libvirt_private.syms
> +++ b/src/libvirt_private.syms
> @@ -1359,6 +1359,7 @@ virBitmapNewCopy;
>  virBitmapNewData;
>  virBitmapNewEmpty;
>  virBitmapNewQuiet;
> +virBitmapNewString;
>  virBitmapNextClearBit;
>  virBitmapNextSetBit;
>  virBitmapOverlaps;
> diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c
> index 0a7d3452b90c..02d1f264d859 100644
> --- a/src/util/virbitmap.c
> +++ b/src/util/virbitmap.c
> @@ -36,6 +36,7 @@
>  #include "c-ctype.h"
>  #include "count-one-bits.h"
>  #include "virstring.h"
> +#include "virutil.h"
>  #include "virerror.h"
>  
>  #define VIR_FROM_THIS VIR_FROM_NONE
> @@ -1068,6 +1069,41 @@ virBitmapCountBits(virBitmapPtr bitmap)
>      return ret;
>  }
>  

Two blank lines for new functions

> +/**
> + * virBitmapNewString:
> + * @string: the string

to be converted into a bitmap

> + *
> + * Allocate a bitmap from a string of hexadecimal data.
> + *
> + * Returns a pointer to the allocated bitmap or NULL if
> + * memory cannot be allocated.
> + */
> +virBitmapPtr
> +virBitmapNewString(const char *string)
> +{
> +    virBitmapPtr bitmap;
> +    size_t i = 0;
> +    size_t len = strlen(string);
> +
> +    if (strspn(string, "0123456789abcdefABCDEF") != len) {
> +        virReportError(VIR_ERR_INVALID_ARG,
> +                       _("Invalid hexadecimal string '%s'"), string);
> +        return NULL;
> +    }
> +
> +    bitmap = virBitmapNew(len * 4);
> +    if (!bitmap)
> +        return NULL;
> +
> +    for (i = 0; i < len; i++) {
> +        unsigned long nibble = virHexToBin(string[len - i - 1]);
> +        nibble <<= VIR_BITMAP_BIT_OFFSET(i * 4);
> +        bitmap->map[VIR_BITMAP_UNIT_OFFSET(i * 4)] |= nibble;
> +    }
> +
> +    return bitmap;
> +}
> +

Two blank lines

>  /**
>   * virBitmapDataFormat:
>   * @data: the data
> diff --git a/src/util/virbitmap.h b/src/util/virbitmap.h
> index 02acb7519d37..e964a3edc9cb 100644
> --- a/src/util/virbitmap.h
> +++ b/src/util/virbitmap.h
> @@ -80,6 +80,10 @@ 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;
>  
> +virBitmapPtr
> +virBitmapNewString(const char *string)
> +    ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK;
> +
>  char *virBitmapToString(virBitmapPtr bitmap, bool prefix, bool trim)
>      ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK;
>  
> diff --git a/tests/virbitmaptest.c b/tests/virbitmaptest.c
> index 488796719dd9..3ea63e1295af 100644
> --- a/tests/virbitmaptest.c
> +++ b/tests/virbitmaptest.c
> @@ -663,6 +663,42 @@ test12(const void *opaque ATTRIBUTE_UNUSED)
>      return ret;
>  }
>  
> +
> +/* virBitmap(New/To)String */
> +static int
> +test13(const void *opaque ATTRIBUTE_UNUSED)
> +{
> +    virBitmapPtr map = NULL;
> +    const char *strings[] = { "1234feebee", "000c0fefe" };

hahahaha, but you're missing the 'v' (covfefe)

> +    char *str = NULL;
> +    size_t i = 0;
> +    int ret = -1;
> +
> +    for (i = 0; i < ARRAY_CARDINALITY(strings); i++) {
> +        map = virBitmapNewString(strings[i]);
> +        str = virBitmapToString(map, false, true);
> +
> +        if (!map || !str)
> +            goto cleanup;
> +
> +        if (STRNEQ(strings[i], str)) {
> +            fprintf(stderr, "\n expected bitmap string '%s' actual string "
> +                    "'%s'\n", NULLSTR(strings[i]), NULLSTR(str));

Don't believe either needs NULLSTR.... If they did STRNEQ would probably
need NULLABLE too ;-)... Still @str cannot be NULL if we get here and if
strings[i] is NULL, then something very strange happened.

> +            goto cleanup;
> +        }
> +
> +        VIR_FREE(str);
> +        virBitmapFree(map);
> +        map = NULL;
> +    }
> +
> +    ret = 0;
> + cleanup:
> +    VIR_FREE(str);
> +    virBitmapFree(map);
> +    return ret;
> +}
> +
>  #undef TEST_MAP
>  
>  
> @@ -711,6 +747,8 @@ mymain(void)
>  
>      if (virTestRun("test12", test12, NULL) < 0)
>          ret = -1;
> +    if (virTestRun("test13", test13, NULL) < 0)
> +        ret = -1;
>  
>      return ret;
>  }
>