On 11/23/22 15:01, Ilya Maximets wrote:
> 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 <[email protected]>
>>> Co-authored-by: Dumitru Ceara <[email protected]>
>>> Signed-off-by: Dumitru Ceara <[email protected]>
>>> Signed-off-by: Ilya Maximets <[email protected]>
>>> ---
>>> 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).
Applied now and backported down to 2.13.
Best regards, Ilya Maximets.
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
