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

[Ján Tomko]

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 at 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 at redhat.com>

Jano
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20201005/51c760ae/attachment-0001.sig>