On 11/21/22 16:54, Adrian Moreno wrote:
>
>
> On 11/4/22 15:25, Ilya Maximets wrote:
>> Some macros for rculist have no users and there are no unit tests
>> specific to that library as well, so broken code wasn't spotted
>> while updating to multi-variable iterators.
>>
>> Fixing multiple problems like missing commas, parenthesis, incorrect
>> variable and macro names.
>>
>> Fixes: d293965d7b06 ("rculist: use multi-variable helpers for loop macros.")
>> Reported-by: Subrata Nath <subrata.n...@nokia.com>
>> Co-authored-by: Dumitru Ceara <dce...@redhat.com>
>> Signed-off-by: Dumitru Ceara <dce...@redhat.com>
>> Signed-off-by: Ilya Maximets <i.maxim...@ovn.org>
>> ---
>> lib/rculist.h | 18 +++++++++---------
>> 1 file changed, 9 insertions(+), 9 deletions(-)
>>
>> diff --git a/lib/rculist.h b/lib/rculist.h
>> index c0d77acf9..9bb8cbf3e 100644
>> --- a/lib/rculist.h
>> +++ b/lib/rculist.h
>> @@ -380,18 +380,18 @@ rculist_is_singleton_protected(const struct rculist
>> *list)
>> #define RCULIST_FOR_EACH_REVERSE_PROTECTED(ITER, MEMBER, RCULIST)
>> \
>> for (INIT_MULTIVAR(ITER, MEMBER, (RCULIST)->prev, struct rculist);
>> \
>> CONDITION_MULTIVAR(ITER, MEMBER, ITER_VAR(ITER) != (RCULIST));
>> \
>> - UPDATE_MULTIVAR(ITER, ITER_VAR(VAR).prev))
>> + UPDATE_MULTIVAR(ITER, ITER_VAR(ITER)->prev))
>> #define RCULIST_FOR_EACH_REVERSE_PROTECTED_CONTINUE(ITER, MEMBER,
>> RCULIST) \
>> for (INIT_MULTIVAR(ITER, MEMBER, (ITER)->MEMBER.prev, struct rculist);
>> \
>> CONDITION_MULTIVAR(ITER, MEMBER, ITER_VAR(ITER) != (RCULIST));
>> \
>> - UPDATE_MULTIVAR(ITER, ITER_VAR(VAR).prev))
>> + UPDATE_MULTIVAR(ITER, ITER_VAR(ITER)->prev))
>>
>
> There's another hidden problem with the REVERSE iterators that has not popped
> up yet: They access 'prev' member directly which will not compile when using
> clang's thread-safety macros because of a fake mutex specifically added to
> avoid it.
> Since the macros are PROTECTED it should be OK to use
> rculist_back_protected() instead.
Hmm, interesting.
>
> I have written a unit test for rculist that I was planning to post soon. If
> you prefer I can fix this at the same time.
If you can fix that in your patch that would be easier, I think.
I'll try to merge the current fix somewhere soon (Just got back
from PTO today).
Best regards, Ilya Maximets.
>
>
>> #define RCULIST_FOR_EACH_PROTECTED(ITER, MEMBER, RCULIST)
>> \
>> for (INIT_MULTIVAR(ITER, MEMBER, rculist_next_protected(RCULIST),
>> \
>> struct rculist);
>> \
>> CONDITION_MULTIVAR(ITER, MEMBER, ITER_VAR(ITER) != (RCULIST));
>> \
>> - UPDATE_MULTIVAR(ITER, rculist_next_protected(ITER_VAR(ITER)))
>> \
>> + UPDATE_MULTIVAR(ITER, rculist_next_protected(ITER_VAR(ITER))))
>> \
>> #define RCULIST_FOR_EACH_SAFE_SHORT_PROTECTED(ITER, MEMBER, RCULIST)
>> \
>> for (INIT_MULTIVAR_SAFE_SHORT(ITER, MEMBER,
>> \
>> @@ -399,18 +399,18 @@ rculist_is_singleton_protected(const struct rculist
>> *list)
>> struct rculist);
>> \
>> CONDITION_MULTIVAR_SAFE_SHORT(ITER, MEMBER,
>> \
>> ITER_VAR(ITER) != (RCULIST),
>> \
>> - ITER_NEXT_VAR(ITER) = rculist_next_protected(ITER_VAR(VAR)));
>> \
>> - UPDATE_MULTIVAR_SHORT(ITER))
>> + ITER_NEXT_VAR(ITER) = rculist_next_protected(ITER_VAR(ITER)));
>> \
>> + UPDATE_MULTIVAR_SAFE_SHORT(ITER))
>> #define RCULIST_FOR_EACH_SAFE_LONG_PROTECTED(ITER, NEXT, MEMBER,
>> RCULIST) \
>> for (INIT_MULTIVAR_SAFE_LONG(ITER, NEXT, MEMBER,
>> \
>> - rculist_next_protected(RCULIST)
>> \
>> + rculist_next_protected(RCULIST),
>> \
>> struct rculist);
>> \
>> - CONDITION_MULTIVAR_SAFE_LONG(VAR, NEXT, MEMBER
>> \
>> + CONDITION_MULTIVAR_SAFE_LONG(ITER, NEXT, MEMBER,
>> \
>> ITER_VAR(ITER) != (RCULIST),
>> \
>> - ITER_VAR(NEXT) = rculist_next_protected(ITER_VAR(VAR)),
>> \
>> + ITER_VAR(NEXT) = rculist_next_protected(ITER_VAR(ITER)),
>> \
>> ITER_VAR(NEXT) != (RCULIST));
>> \
>> - UPDATE_MULTIVAR_LONG(ITER))
>> + UPDATE_MULTIVAR_SAFE_LONG(ITER, NEXT))
>> #define RCULIST_FOR_EACH_SAFE_PROTECTED(...)
>> \
>> OVERLOAD_SAFE_MACRO(RCULIST_FOR_EACH_SAFE_LONG_PROTECTED,
>> \
>
> Thanks
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
