On 20/7/26 10:31, Peter Maydell wrote:
On Mon, 20 Jul 2026 at 08:49, Philippe Mathieu-Daudé
<[email protected]> wrote:

Hi Brian,

some random thoughts...

On 15/7/26 20:56, Brian Cain wrote:
set_bit32()/test_bit32()/etc already let devices operate on
guest-visible uint32_t register arrays without depending on the
host's 'unsigned long' size. find_first_bit() has no such
equivalent, which pushes callers towards casting a uint32_t array
to 'unsigned long *'.

Add find_first_bit32(), implemented the same way as find_first_bit().

Reviewed-by: Pierrick Bouvier <[email protected]>
Signed-off-by: Brian Cain <[email protected]>
---
   include/qemu/bitops.h | 30 ++++++++++++++++++++++++++----
   1 file changed, 26 insertions(+), 4 deletions(-)


+/**
+ * find_first_bit32 - find the first set bit in a memory region
+ * @addr: The address to start the search at

s/addr/ptr/?

+ * @size: The maximum size to search

Pre-existing, it is not clear this is expressed in bits. Neither if
we expect it to be a multiple of 32, or @addr to be 32-bit aligbed.

Maybe rename as @last_searched_bit?

Could we please stop bikeshedding this function? It's
a simple copy of the existing find_first_bit() but
with the type changed from 'unsigned long' to 'uint32_t'.
The comments and parameter names on that existing function
have been fine for years, and having the new function
pick different names and comment phrasing seems unnecessary
and obscures the parallels between the two functions.

Argument renaming of course implies renaming the other companion
methods. You taught me to add public APIs with good descriptions
when possible, I thought your advice was worth to other contributors.
I'm not considering this review comment as bikeshedding. Anyway,
thanks for your review and advice. I don't object if Brian keeps
the current names or rename as @a, @b, @foo, @bar.

Thanks.

Reply via email to