Re: [libvirt] [PATCH 1/3] util: Add a helper to check if all bits of a bitmap are clear

2013-04-02 Thread John Ferlan 
On 04/02/2013 01:42 AM, Osier Yang wrote:
 ---
  src/libvirt_private.syms |  1 +
  src/util/virbitmap.c | 30 ++
  src/util/virbitmap.h |  3 +++
  3 files changed, 34 insertions(+)
 

Since there already is a virBitmapIsAllSet() - why isn't it used?  I
see callers which do if (virBitmapIsAllSet(...) and if
(!virBitmapIsAllSet(...).

If you're going to have a AllClear(), then why not change those !
callers to use AllClear()...

I only wonder about the last comparison - it's the -1 logic that
throws me off especially since the IsAllSet() code is doing a comparison.

It also stands to reason that tests/virbitmaptest.c could add new tests
to ensure you did get the logic right.

John

 diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
 index 96eea0a..35ac957 100644
 --- a/src/libvirt_private.syms
 +++ b/src/libvirt_private.syms
 @@ -1048,6 +1048,7 @@ virBitmapEqual;
  virBitmapFormat;
  virBitmapFree;
  virBitmapGetBit;
 +virBitmapIsAllClear;
  virBitmapIsAllSet;
  virBitmapNew;
  virBitmapNewCopy;
 diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c
 index 21509ac..99a8572 100644
 --- a/src/util/virbitmap.c
 +++ b/src/util/virbitmap.c
 @@ -591,6 +591,36 @@ bool virBitmapIsAllSet(virBitmapPtr bitmap)
  }
  
  /**
 + * virBitmapIsAllClear:
 + * @bitmap: the bitmap to check
 + *
 + * check if all bits in @bitmap are clear
 + */
 +bool virBitmapIsAllClear(virBitmapPtr bitmap)
 +{
 +int i;
 +int unusedBits;
 +size_t sz;
 +
 +unusedBits = bitmap-map_len * VIR_BITMAP_BITS_PER_UNIT - 
 bitmap-max_bit;
 +
 +sz = bitmap-map_len;
 +if (unusedBits  0)
 +sz--;
 +
 +for (i = 0; i  sz; i++)
 +if (bitmap-map[i] != 0)
 +return false;
 +
 +if (unusedBits  0) {
 +if ((bitmap-map[sz]  ((1UL  (VIR_BITMAP_BITS_PER_UNIT - 
 unusedBits)) - 1)))
 +return false;
 +}
 +
 +return true;
 +}
 +
 +/**
   * virBitmapNextSetBit:
   * @bitmap: the bitmap
   * @pos: the position after which to search for a set bit
 diff --git a/src/util/virbitmap.h b/src/util/virbitmap.h
 index 044c7a6..b682523 100644
 --- a/src/util/virbitmap.h
 +++ b/src/util/virbitmap.h
 @@ -100,6 +100,9 @@ void virBitmapClearAll(virBitmapPtr bitmap)
  bool virBitmapIsAllSet(virBitmapPtr bitmap)
  ATTRIBUTE_NONNULL(1);
  
 +bool virBitmapIsAllClear(virBitmapPtr bitmap)
 +ATTRIBUTE_NONNULL(1);
 +
  ssize_t virBitmapNextSetBit(virBitmapPtr bitmap, ssize_t pos)
  ATTRIBUTE_NONNULL(1);
  
 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 1/3] util: Add a helper to check if all bits of a bitmap are clear

2013-04-02 Thread Daniel P. Berrange 
On Tue, Apr 02, 2013 at 06:52:22AM -0400, John Ferlan wrote:
 On 04/02/2013 01:42 AM, Osier Yang wrote:
  ---
   src/libvirt_private.syms |  1 +
   src/util/virbitmap.c | 30 ++
   src/util/virbitmap.h |  3 +++
   3 files changed, 34 insertions(+)
  
 
 Since there already is a virBitmapIsAllSet() - why isn't it used?  I
 see callers which do if (virBitmapIsAllSet(...) and if
 (!virBitmapIsAllSet(...).
 
 If you're going to have a AllClear(), then why not change those !
 callers to use AllClear()...

!virBitmapIsAllSet() is not the same as virBitmapIsAllClear().

!virBitmapIsAllSet() allows for [0 - (n-1)] bits to be set,
whereas virBitmapIsAllClear() only allows for 0 bits to be
set.

Daniel
-- 
|: http://berrange.com  -o-http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org  -o- http://virt-manager.org :|
|: http://autobuild.org   -o- http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org   -o-   http://live.gnome.org/gtk-vnc :|

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 1/3] util: Add a helper to check if all bits of a bitmap are clear

2013-04-02 Thread Osier Yang 

On 02/04/13 18:52, John Ferlan wrote:

On 04/02/2013 01:42 AM, Osier Yang wrote:

---
  src/libvirt_private.syms |  1 +
  src/util/virbitmap.c | 30 ++
  src/util/virbitmap.h |  3 +++
  3 files changed, 34 insertions(+)


Since there already is a virBitmapIsAllSet() - why isn't it used?  I
see callers which do if (virBitmapIsAllSet(...) and if
(!virBitmapIsAllSet(...).


I want to check if the bitmap is all zero. Obviously !virBitmapIsAllSet
can't do it.



If you're going to have a AllClear(), then why not change those !
callers to use AllClear()...

I only wonder about the last comparison - it's the -1 logic that
throws me off especially since the IsAllSet() code is doing a comparison.

It also stands to reason that tests/virbitmaptest.c could add new tests
to ensure you did get the logic right.


Agreed. I didn't notice this. Will add.

Osier

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [PATCH 1/3] util: Add a helper to check if all bits of a bitmap are clear

2013-04-02 Thread Eric Blake 
On 04/01/2013 11:42 PM, Osier Yang wrote:
 ---
  src/libvirt_private.syms |  1 +
  src/util/virbitmap.c | 30 ++
  src/util/virbitmap.h |  3 +++
  3 files changed, 34 insertions(+)
 

 +bool virBitmapIsAllClear(virBitmapPtr bitmap)
 +{
 +int i;
 +int unusedBits;
 +size_t sz;
 +
 +unusedBits = bitmap-map_len * VIR_BITMAP_BITS_PER_UNIT - 
 bitmap-max_bit;
 +
 +sz = bitmap-map_len;
 +if (unusedBits  0)
 +sz--;
 +
 +for (i = 0; i  sz; i++)
 +if (bitmap-map[i] != 0)
 +return false;
 +
 +if (unusedBits  0) {
 +if ((bitmap-map[sz]  ((1UL  (VIR_BITMAP_BITS_PER_UNIT - 
 unusedBits)) - 1)))
 +return false;

You are being careful to avoid assuming any state about the bits in the
tail beyond the bitmap size.  But I thought our code was already careful
to ensure that the tail bits are always 0.  Therefore, you should be
able to just check that the entire bitmap-map is 0, without
special-casing the tail.

-- 
Eric Blake   eblake redhat com+1-919-301-3266
Libvirt virtualization library http://libvirt.org



signature.asc
Description: OpenPGP digital signature
--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

[libvirt] [PATCH 1/3] util: Add a helper to check if all bits of a bitmap are clear

2013-04-01 Thread Osier Yang 
---
 src/libvirt_private.syms |  1 +
 src/util/virbitmap.c | 30 ++
 src/util/virbitmap.h |  3 +++
 3 files changed, 34 insertions(+)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 96eea0a..35ac957 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1048,6 +1048,7 @@ virBitmapEqual;
 virBitmapFormat;
 virBitmapFree;
 virBitmapGetBit;
+virBitmapIsAllClear;
 virBitmapIsAllSet;
 virBitmapNew;
 virBitmapNewCopy;
diff --git a/src/util/virbitmap.c b/src/util/virbitmap.c
index 21509ac..99a8572 100644
--- a/src/util/virbitmap.c
+++ b/src/util/virbitmap.c
@@ -591,6 +591,36 @@ bool virBitmapIsAllSet(virBitmapPtr bitmap)
 }
 
 /**
+ * virBitmapIsAllClear:
+ * @bitmap: the bitmap to check
+ *
+ * check if all bits in @bitmap are clear
+ */
+bool virBitmapIsAllClear(virBitmapPtr bitmap)
+{
+int i;
+int unusedBits;
+size_t sz;
+
+unusedBits = bitmap-map_len * VIR_BITMAP_BITS_PER_UNIT - bitmap-max_bit;
+
+sz = bitmap-map_len;
+if (unusedBits  0)
+sz--;
+
+for (i = 0; i  sz; i++)
+if (bitmap-map[i] != 0)
+return false;
+
+if (unusedBits  0) {
+if ((bitmap-map[sz]  ((1UL  (VIR_BITMAP_BITS_PER_UNIT - 
unusedBits)) - 1)))
+return false;
+}
+
+return true;
+}
+
+/**
  * virBitmapNextSetBit:
  * @bitmap: the bitmap
  * @pos: the position after which to search for a set bit
diff --git a/src/util/virbitmap.h b/src/util/virbitmap.h
index 044c7a6..b682523 100644
--- a/src/util/virbitmap.h
+++ b/src/util/virbitmap.h
@@ -100,6 +100,9 @@ void virBitmapClearAll(virBitmapPtr bitmap)
 bool virBitmapIsAllSet(virBitmapPtr bitmap)
 ATTRIBUTE_NONNULL(1);
 
+bool virBitmapIsAllClear(virBitmapPtr bitmap)
+ATTRIBUTE_NONNULL(1);
+
 ssize_t virBitmapNextSetBit(virBitmapPtr bitmap, ssize_t pos)
 ATTRIBUTE_NONNULL(1);
 
-- 
1.8.1.4

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list