Yun, could you please stop top-posting and excessive trimming in the thread?
On Thu, Dec 3, 2020 at 1:47 AM Yun Levi <ppbuk5246@xxxxxxxxx> wrote:
> > Either just make the return type of all find_prev/find_last be signed
> > int and use -1 as the sentinel to indicate "no such position exists", so
> > the loop condition would be foo >= 0. Or, change the condition from
> > "stop if we get the size returned" to "only continue if we get something
> > strictly less than the size we passed in (i.e., something which can
> > possibly be a valid bit index). In the latter case, both (unsigned)-1
> > aka UINT_MAX and the actual size value passed work equally well as a
> > sentinel.
> >
> > If one uses UINT_MAX, a for_each_bit_reverse() macro would just be
> > something like
> >
> > for (i = find_last_bit(bitmap, size); i < size; i =
> > find_last_bit(bitmap, i))
> >
> > if one wants to use the size argument as the sentinel, the caller would
> > have to supply a scratch variable to keep track of the last i value:
> >
> > for (j = size, i = find_last_bit(bitmap, j); i < j; j = i, i =
> > find_last_bit(bitmap, j))
> >
> > which is probably a little less ergonomic.
> >
> > Rasmus
I would prefer to avoid changing the find*bit() semantics. As for now,
if any of find_*_bit()
finds nothing, it returns the size of the bitmap it was passed.
Changing this for
a single function would break the consistency, and may cause problems
for those who
rely on existing behaviour.
Passing non-positive size to find_*_bit() should produce undefined
behaviour, because we cannot dereference a pointer to the bitmap in
this case; this is most probably a sign of a problem on a caller side
anyways.
Let's keep this logic unchanged?
> Actually Because I want to avoid the modification of return type of
> find_last_*_bit for new sentinel,
> I add find_prev_*_bit.
> the big difference between find_last_bit and find_prev_bit is
> find_last_bit doesn't check the size bit and use sentinel with size.
> but find_prev_bit check the offset bit and use sentinel with size
> which passed by another argument.
> So if we use find_prev_bit, we could have a clear iteration if
> using find_prev_bit like.
>
> #define for_each_set_bit_reverse(bit, addr, size) \
> for ((bit) = find_last_bit((addr), (size)); \
> (bit) < (size); \
> (bit) = find_prev_bit((addr), (size), (bit - 1)))
>
> #define for_each_set_bit_from_reverse(bit, addr, size) \
> for ((bit) = find_prev_bit((addr), (size), (bit)); \
> (bit) < (size); \
> (bit) = find_prev_bit((addr), (size), (bit - 1)))
>
> Though find_prev_*_bit / find_last_*_bit have the same functionality.
> But they also have a small difference.
> I think this small this small difference doesn't make some of
> confusion to user but it help to solve problem
> with a simple way (just like the iteration above).
>
> So I think I need, find_prev_*_bit series.
>
> Am I missing anything?
>
> Thanks.
>
> Levi.
As you said, find_last_bit() and proposed find_prev_*_bit() have the
same functionality.
If you really want to have find_prev_*_bit(), could you please at
least write it using find_last_bit(), otherwise it would be just a
blottering.
Regarding reverse search, we can probably do like this (not tested,
just an idea):
#define for_each_set_bit_reverse(bit, addr, size) \
for ((bit) = find_last_bit((addr), (size)); \
(bit) < (size); \
(size) = (bit), (bit) = find_last_bit((addr), (bit)))
Thanks,
Yury
