On a Monday in 2020, Peter Krempa wrote:
On Fri, Oct 02, 2020 at 17:44:13 +0200, Ján Tomko wrote:
On a Friday in 2020, Peter Krempa wrote:
On Fri, Oct 02, 2020 at 13:17:17 +0200, Ján Tomko wrote:
On a Friday in 2020, Peter Krempa wrote:
Clarify which bit is considered most significant in the bitmap and resulting string. Also be explicit that it's a hex string.
Signed-off-by: Peter Krempa <pkrempa@redhat.com> --- src/util/virbitmap.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-)
diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c index ad5213f216..fcb8e1101a 100644 --- a/src/util/virbitmap.c +++ b/src/util/virbitmap.c @@ -328,7 +328,9 @@ virBitmapGetBit(virBitmapPtr bitmap, * virBitmapToString: * @bitmap: Pointer to bitmap * - * Convert @bitmap to printable string. + * Convert @bitmap to printable hexadecimal string representation. Note that bit + * with highest position/index in @bitmap are considered as most significant bit + * in the output string.
the bits ... are considered or the bit ... is considered
oops, I've rewrote it halfway through ...
would mentioning that it is printed at the leftmost position be clearer?
Well, the thing is that the leftmost digit in the output string represents more than one bit since it's hex. I thought about some wordign but couldn't come up with anything more appropriate.
We could do: 'is considered as the most significant bit of the number represented by the output string', or just 'most significant bit of the output number". That way the reader knows it's a number and the semantics of the bit are then implicit.
Either of those LGTM
I'll go with:
diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c index ad5213f216..ed28427736 100644 --- a/src/util/virbitmap.c +++ b/src/util/virbitmap.c @@ -328,7 +328,9 @@ virBitmapGetBit(virBitmapPtr bitmap, * virBitmapToString: * @bitmap: Pointer to bitmap * - * Convert @bitmap to printable string. + * Convert @bitmap to a number where the bit with highest position/index in + * @bitmap represents the most significant bit and return the number in form + * of a hexadecimal string. * * Returns pointer to the string or NULL on error. */ @@ -1117,10 +1119,14 @@ virBitmapCountBits(virBitmapPtr bitmap) * virBitmapNewString: * @string: the string to be converted to a bitmap * - * Allocate a bitmap from a string of hexadecimal data. + * Allocate a bitmap and populate it from @string representing a number in + * hexadecimal format. Note that the most significant bit of the number + * represented by @string will correspond to the highest index/position in the + * bitmap. The size of the returned bitmap corresponds to 4 * the length of + * @string. * - * Returns a pointer to the allocated bitmap or NULL if - * memory cannot be allocated. + * Returns a pointer to the allocated bitmap or NULL and reports an error if + * @string can't be converted. */ virBitmapPtr virBitmapNewString(const char *string)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano