diff options
author | Thomas Haller <thaller@redhat.com> | 2018-11-13 19:01:54 +0100 |
---|---|---|
committer | Thomas Haller <thaller@redhat.com> | 2018-11-13 19:04:34 +0100 |
commit | e1413111a79da9554be32ea1cabc3f88c0893d8c (patch) | |
tree | 8d9ae6d4c56251600486557dce3d877bd0dcf29d | |
parent | eb9f950a330212d4780971d269423b0d90c51c5c (diff) | |
download | NetworkManager-e1413111a79da9554be32ea1cabc3f88c0893d8c.tar.gz |
core: minor cleanup of initializing nm_utils_get_testing()
- add a commnt about thread-safety.
- minor refactoring initializing the value in nm_utils_get_testing().
Instead of returning the flags we just set, go back to the begin
and re-read the value (which must be initialized by now). No big
difference, but feels a bit nicer to me.
-rw-r--r-- | src/nm-core-utils.c | 23 |
1 files changed, 17 insertions, 6 deletions
diff --git a/src/nm-core-utils.c b/src/nm-core-utils.c index 61d484f3ac..3d7047c3c9 100644 --- a/src/nm-core-utils.c +++ b/src/nm-core-utils.c @@ -52,6 +52,17 @@ #endif G_STATIC_ASSERT (sizeof (NMUtilsTestFlags) <= sizeof (int)); + +/* we read _nm_utils_testing without memory barrier. This is thread-safe, + * because the static variable is initialized to zero, and only reset + * once to a non-zero value (via g_atomic_int_compare_and_exchange()). + * + * Since there is only one integer that contains the data, there is no + * caching problem reading this (atomic int) variable without + * synchronization/memory-barrier. Contrary to a double-checked locking, + * where one needs a memory barrier to read the variable and ensure + * that also the related data is coherent in cache. Here there is no + * related data. */ static int _nm_utils_testing = 0; gboolean @@ -70,6 +81,7 @@ nm_utils_get_testing () { NMUtilsTestFlags flags; +again: flags = (NMUtilsTestFlags) _nm_utils_testing; if (flags != NM_UTILS_TEST_NONE) { /* Flags already initialized. Return them. */ @@ -82,12 +94,11 @@ nm_utils_get_testing () if (g_test_initialized ()) flags |= _NM_UTILS_TEST_GENERAL; - if (g_atomic_int_compare_and_exchange (&_nm_utils_testing, 0, (int) flags)) { - /* Done. We set it. */ - return flags & NM_UTILS_TEST_ALL; - } - /* It changed in the meantime (??). Re-read the value. */ - return ((NMUtilsTestFlags) _nm_utils_testing) & NM_UTILS_TEST_ALL; + g_atomic_int_compare_and_exchange (&_nm_utils_testing, 0, (int) flags); + + /* regardless of whether we won the race of initializing _nm_utils_testing, + * go back and read the value again. It must be non-zero by now. */ + goto again; } void |