[PATCH 03/43] conf: nwfilter_ipaddrmap: convert virMutex to GMutex

[Laine Stump]

On 4/15/20 8:43 AM, Pavel Mores wrote:
> On Sat, Apr 11, 2020 at 08:15:15PM +0200, Rafael Fonseca wrote:
>> On Fri, Apr 10, 2020 at 7:06 PM Laine Stump <laine at laine.org> wrote:
>>>> Beyond that, why not just use the G_*() macros for *all* locks in
>>>> libvirt instead of only using them in cases of static locks? It seems
>>>> strange to use the convenience macros in some cases and the raw
>>>> functions in others. (I'm looking at this with 0 experience using the
>>>> Glib locks, so maybe there's something basic I'm just not aware of -
>>>> maybe something necessary is missing from the G_LOCK() macros?).
>>>
>>> Okay, I already see that the G_LOCK macros don't cover everything - no
>>> recursive mutexes and no RW mutexes for example. Too bad - it would be
>>> good to be consistent.
>> Yes, that's one issue. Another is: how do you use those macros with
>> locks inside structs? You can't do `G_LOCK(obj->parent.lock)` because
>> it'll result in `g_mutex_lock(&g__obj->parent.lock_lock)` which is
>> wrong. You'd have to use the raw function
>> `g_mutex_lock(obj->parent.LOCK_NAME(lock))` anyway, which imho, is
>> even worse than `g_mutex_lock(&obj->parent.lock)`. The same issue
>> happens when using mutexes with conditions: `g_cond_wait(cond,
>> obj->parent.LOCK_NAME(lock))
>> ` instead of just `g_cond_wait(cond, obj->parent.lock)`. So they work
>> better for statically-defined locks
>>
>> I don't mind doing whichever you guys prefer, just let me know.
> Looking at the source code, the name mangling is pretty much all that the
> G_LOCK_DEFINE* macros do.  So - besides some logging - the only advantage to
> using them is that you don't have to mangle the lock names manually and can use
> names of existing variables as the macros' arguments.
>
> Considering the above, I'd say either use the macros and don't mangle the lock
> names in their arguments manually, or don't use the macros.  If consistent
> style is a priority I'd lean towards raw functions


I agree with this - consistency is king :-)


>   - unlike the macros, they
> can be used everywhere, and having to mangle the lock names by hand doesn't
> seem a huge burden to me.  We do loose the logging that the macros do but in my
> experience, mutex logging often doesn't turn out as useful in practice as it
> might first appear...
>
> 	pvl
>