On 4/10/20 1:01 PM, Laine Stump wrote:
On 4/10/20 9:54 AM, Rafael Fonseca wrote:
> Signed-off-by: Rafael Fonseca <r4f4rfs(a)gmail.com>
> ---
> src/conf/nwfilter_ipaddrmap.c | 14 +++++++-------
> 1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/src/conf/nwfilter_ipaddrmap.c
> b/src/conf/nwfilter_ipaddrmap.c
> index 672a58be3a..3c1927f9ff 100644
> --- a/src/conf/nwfilter_ipaddrmap.c
> +++ b/src/conf/nwfilter_ipaddrmap.c
> @@ -32,7 +32,7 @@
> #define VIR_FROM_THIS VIR_FROM_NWFILTER
> -static virMutex ipAddressMapLock = VIR_MUTEX_INITIALIZER;
> +G_LOCK_DEFINE_STATIC(ipAddressMapLock);
The documentation for the G_LOCK() macros says that the name provided
in the args will be mangled, so you can just use the same name as the
object you're going to lock, e.g.:
G_LOCK_DEFINE_STATIC(ipAddressMap);
instead of "ipAddressMapLock". The upside is that's less typing, and
if you do a keyword search you'll find all the places where
ipAddressMap is locked/unlocked along with its uses. The downside
would be that you couldn't as easily do a keyword search for just the
lock manipulation (you'd need to use a regex to search for
"G_.*ipAddressMap" or something like that); I still kind of like the
idea of using the exact same name, just because it encourages
consistency.
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.