[PATCH v3 02/10] hbitmap: move hbitmap_iter_next_word to hbitmap.c

Vladimir Sementsov-Ogievskiy posted 10 patches 6 years, 1 month ago
Maintainers: John Snow <jsnow@redhat.com>, Max Reitz <mreitz@redhat.com>, Kevin Wolf <kwolf@redhat.com>, Eric Blake <eblake@redhat.com>, Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
There is a newer version of this series
[PATCH v3 02/10] hbitmap: move hbitmap_iter_next_word to hbitmap.c
Posted by Vladimir Sementsov-Ogievskiy 6 years, 1 month ago
The function is definitely internal (it's not used by third party and
it has complicated interface). Move it to .c file.

Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
---
 include/qemu/hbitmap.h | 30 ------------------------------
 util/hbitmap.c         | 29 +++++++++++++++++++++++++++++
 2 files changed, 29 insertions(+), 30 deletions(-)

diff --git a/include/qemu/hbitmap.h b/include/qemu/hbitmap.h
index 1bf944ca3d..ab227b117f 100644
--- a/include/qemu/hbitmap.h
+++ b/include/qemu/hbitmap.h
@@ -362,34 +362,4 @@ void hbitmap_free_meta(HBitmap *hb);
  */
 int64_t hbitmap_iter_next(HBitmapIter *hbi);
 
-/**
- * hbitmap_iter_next_word:
- * @hbi: HBitmapIter to operate on.
- * @p_cur: Location where to store the next non-zero word.
- *
- * Return the index of the next nonzero word that is set in @hbi's
- * associated HBitmap, and set *p_cur to the content of that word
- * (bits before the index that was passed to hbitmap_iter_init are
- * trimmed on the first call).  Return -1, and set *p_cur to zero,
- * if all remaining words are zero.
- */
-static inline size_t hbitmap_iter_next_word(HBitmapIter *hbi, unsigned long *p_cur)
-{
-    unsigned long cur = hbi->cur[HBITMAP_LEVELS - 1];
-
-    if (cur == 0) {
-        cur = hbitmap_iter_skip_words(hbi);
-        if (cur == 0) {
-            *p_cur = 0;
-            return -1;
-        }
-    }
-
-    /* The next call will resume work from the next word.  */
-    hbi->cur[HBITMAP_LEVELS - 1] = 0;
-    *p_cur = cur;
-    return hbi->pos;
-}
-
-
 #endif
diff --git a/util/hbitmap.c b/util/hbitmap.c
index 7f9b3e0cd7..a368dc5ef7 100644
--- a/util/hbitmap.c
+++ b/util/hbitmap.c
@@ -298,6 +298,35 @@ uint64_t hbitmap_count(const HBitmap *hb)
     return hb->count << hb->granularity;
 }
 
+/**
+ * hbitmap_iter_next_word:
+ * @hbi: HBitmapIter to operate on.
+ * @p_cur: Location where to store the next non-zero word.
+ *
+ * Return the index of the next nonzero word that is set in @hbi's
+ * associated HBitmap, and set *p_cur to the content of that word
+ * (bits before the index that was passed to hbitmap_iter_init are
+ * trimmed on the first call).  Return -1, and set *p_cur to zero,
+ * if all remaining words are zero.
+ */
+static size_t hbitmap_iter_next_word(HBitmapIter *hbi, unsigned long *p_cur)
+{
+    unsigned long cur = hbi->cur[HBITMAP_LEVELS - 1];
+
+    if (cur == 0) {
+        cur = hbitmap_iter_skip_words(hbi);
+        if (cur == 0) {
+            *p_cur = 0;
+            return -1;
+        }
+    }
+
+    /* The next call will resume work from the next word.  */
+    hbi->cur[HBITMAP_LEVELS - 1] = 0;
+    *p_cur = cur;
+    return hbi->pos;
+}
+
 /* Count the number of set bits between start and end, not accounting for
  * the granularity.  Also an example of how to use hbitmap_iter_next_word.
  */
-- 
2.21.0


Re: [PATCH v3 02/10] hbitmap: move hbitmap_iter_next_word to hbitmap.c
Posted by Max Reitz 6 years ago
On 19.12.19 11:03, Vladimir Sementsov-Ogievskiy wrote:
> The function is definitely internal (it's not used by third party and
> it has complicated interface). Move it to .c file.
> 
> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
> ---
>  include/qemu/hbitmap.h | 30 ------------------------------
>  util/hbitmap.c         | 29 +++++++++++++++++++++++++++++
>  2 files changed, 29 insertions(+), 30 deletions(-)

I wonder why you removed the “inline”, but then again we should probably
trust the compiler more than our intuition anyway.

Reviewed-by: Max Reitz <mreitz@redhat.com>

Re: [PATCH v3 02/10] hbitmap: move hbitmap_iter_next_word to hbitmap.c
Posted by Vladimir Sementsov-Ogievskiy 6 years ago
20.01.2020 13:55, Max Reitz wrote:
> On 19.12.19 11:03, Vladimir Sementsov-Ogievskiy wrote:
>> The function is definitely internal (it's not used by third party and
>> it has complicated interface). Move it to .c file.
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>
>> ---
>>   include/qemu/hbitmap.h | 30 ------------------------------
>>   util/hbitmap.c         | 29 +++++++++++++++++++++++++++++
>>   2 files changed, 29 insertions(+), 30 deletions(-)
> 
> I wonder why you removed the “inline”, but then again we should probably
> trust the compiler more than our intuition anyway.

Hmm, somehow, I was sure, that defining "static inline" functions in .c file is
equal to defining them just "static", as compiler will decide inline it or not
it on its own anyway.. May be I'm wrong. At least it's claimed that compiler may
ignore "inline", so it's a hing, and on the other hand it may inline not "inline"
function.

And even if I'm wrong, I agree that in non-critical cases we should trust the
compiler optimization strategy.

> 
> Reviewed-by: Max Reitz <mreitz@redhat.com>
> 


-- 
Best regards,
Vladimir