On Mon, 20 Jul 2026 at 12:46, Philippe Mathieu-Daudé <[email protected]> wrote: > > 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.
Sorry for being a bit grumpy -- it's just that this is the second time around of somebody suggesting a change here to something that's already present in the existing function this is patterned off (and which we borrowed from the Linux kernel originally). -- PMM
