Hi Jonas,
On 10/25/2018 01:55 AM, Jonas Bonn wrote:
Hi Giacinto,
On 25/10/2018 07:03, Giacinto Cifelli wrote:
+static inline struct cb_data *cb_data_ref(struct cb_data *cbd)
+{
+ if (cbd == NULL)
+ return NULL;
+
+ g_atomic_int_inc(&cbd->ref_count);
+
Let me just vent my disagreement with using atomic accessors here again:
i) If I'm a new contributor, unfamiliar with ofono, and I see this then
I think: "oh oh, this is a thread-safe library... better be
careful."... and then I get all confused because nothing else is handled
in a thread-safe manner.
ii) The atomic operations introduce an unnecessary memory barrier.
I _presume_ that the code is written this way because it was inherited
from connman and connman made an early attempt to be thread safe...???
I'm not sure that legacy is worth preserving.
Anyway, enough said on the matter. The implementation is fine aside
from my reservations.
So let me make you happy:
For all new code the policy is that g_atomic should not be used. Once
we switch to ell for oFono 2.x (or even before that) we should have no
instances of g_atomic in the codebase.
If someone wants to start removing some of these now, be my guest.
Regards,
-Denis
_______________________________________________
ofono mailing list
[email protected]
https://lists.ofono.org/mailman/listinfo/ofono
