On 29.05.2018 11:23, Eelco Chaudron wrote:
> On 05/28/2018 03:42 PM, Ilya Maximets wrote:
>> On 28.05.2018 14:06, Eelco Chaudron wrote:
>
> <SNIP>
>>> +unsigned int
>>> +ovsthread_id_init(void)
>>> +{
>>> + static atomic_count next_id = ATOMIC_COUNT_INIT(0);
>>> +
>>> + unsigned int id = *ovsthread_id_get();
>>> +
>>> + if (id == OVSTHREAD_ID_UNSET) {
>> I'm not sure, if we need to check the id here. I think, 'init' function
>> should not be called twice. Also, you're checking the id before calling.
> I don't think checking before calling should be an issue, it will return the
> default.
I'm just a bit uncomfortable with checking inside ovsthread_id_init() because:
1. We always check before the call, except for trivial cases.
2. 'init' function should never be called twice for the same thread because
it's logically incorrect.
>> How about just adding the assert?
> I'm not a big fan of assert, especially in recoverable situations. This way
> calling the function twice will not hurt. Also, this function should never be
> called by the application code directly.
>
> But if assert is a common practice in OVS, I can go ahead and change it.
As I wrote above, the call to 'ovsthread_id_init()' from the same thread twice
is logically wrong. If this happens, it's likely an error on the caller side
and it should be investigated. That's why I think it'll be good to assert here.
>
>
>>> + id = atomic_count_inc(&next_id);
>>> + *ovsthread_id_get() = id;
>>> + }
>>> +
>>> + return id;
>>> +}
>>> +
>>>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
