Re: [PATCH 03/15] virBitmapToString|virBitmapNewString: Clarify semantics of the 'string'

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(a)redhat.com&gt; > > > > --- > > > > 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)