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.

thanks
-- PMM

Reply via email to