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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxxx>
> > > ---
> > > 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@xxxxxxxxxx>

Jano

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux