Coverity wrongly think there is a use after free in the test:
Error: USE_AFTER_FREE (CWE-416): [#def559] [important]
NetworkManager-1.31.90/src/libnm-glib-aux/tests/test-shared-general.c:1305: alias: Assigning: "s1" = "_s". Now both point to the same storage.
NetworkManager-1.31.90/src/libnm-glib-aux/tests/test-shared-general.c:1324: freed_arg: "g_source_unref" frees "s1".
NetworkManager-1.31.90/src/libnm-glib-aux/tests/test-shared-general.c:1330: deref_after_free: Dereferencing freed pointer "s1".
# 1328| s2 = nm_g_source_sentinel_get(0);
# 1329| g_assert(s2 == s1);
# 1330|-> g_assert_cmpint(g_atomic_int_get(&s1->ref_count), >=, 1);
# 1331| }
# 1332| }
Rework the code in the hope to avoid the false warning.
(cherry picked from commit 7825609f1f)
(cherry picked from commit c47c823c9d)
This helper is useful to get a dummy GSource instance that can be
refed, unrefed and destroyed. It can act as a replacement for
a timeout source with infinite timeout.
(cherry picked from commit ce7c28c514)
Fixes: b01a453ca2 ('core: add nm_utils_random_bytes() and use getrandom()')
(cherry picked from commit c127e1becc)
(cherry picked from commit 3de83dd25c)
The previous implementation was just wrong.
Fixes: e1ca3bf7ed ('shared: add nm_strerror_native() to replace strerror() and g_strerror()')
(cherry picked from commit 5bc39d9783)
(cherry picked from commit 963c395cc2)
_nm_thread_local is very neat, but when we allocate resources
we need to make sure that they are destroyed when the thread
exits.
We can use pthread_setspecific() for that, but using it is cumbersome.
Add a helper function to make that simpler.
Also, the number of possible pthread_key_t keys is limited. With this
way, we only need one key in total.
(cherry picked from commit bec4a40437)
(cherry picked from commit e83aad1972)
Since g_string_free() takes an additional argument,
it's not direclty usable with nm_clear_pointer(ptr, g_string_free);
As workaround, add nm_clear_g_string() helper.
(cherry picked from commit 8da91cd85f)
The idea of NM_G_MUTEX_LOCKED() macro is not only to register a mutex
for unlocking (via nm_auto_unlock_g_mutex) but also to lock it at
the same time.
That is a useful helper macro. If you have to lock the mutex yourself,
it makes usage less convenient. At which point you don't need the macro
anymore and you should instead take full control and lock/unlock yourself.
Fix the macro and change behavior. The macro was not used so far, so
it's not a problem.
Fixes: dd33b3a14e ('shared: add nm_auto_unlock_g_mutex and NM_G_MUTEX_LOCKED() helper macros')
(cherry picked from commit 098ac7dbc0)
NMStrBuf's API is all about convenience. When you reset the buffer,
is it convenient to immediately append a new string?
It seems not. Make nm_str_buf_reset() simpler by doing only one thing.
We redefine G_VARIANT_TYPE() because we don't want to pay a run time
check for what should be a simple cast.
But this is strictly tied to glib, and it must only be done after glib.h
was included. Move it to the right place.
We have g_idle_add() and g_timeout_add(). But these return
those odd guint source ids. That is totally pointless. The
only potential benefit is that a guint is only 4 bytes while
a pointer is 8 bytes (on 64 bit systems). Otherwise, it seems
always preferable to have an actual GSource instance instead
of an integer. It also saves the overhead in g_source_remove()
which first needs to do a hash lookup to find the GSource.
A GSource instance would theoretically work with multiple
GMainContext instances, while g_source_remove() only works
wit g_main_context_default().
On the other hand we have helper API like nm_g_idle_source_new()
and nm_g_timeout_source_new(), which is fully flexible and sensible
because it returns a reference to the GSource instance. However, it
is a bit verbose to use in the common case.
Add helper functions that simplify the use and are conceptionally
similar to g_{idle,timeout}_add() (hence the name).
The prepare() step of the GSource called frequently when the outer
GMainContext is preparing.
In the common case we have
- few file descriptors in the inner context to track
- the file descriptors don't change
We also need to consider whether the file descriptors change, because
some book-keeping is necessary when they do. But usually they don't
change.
Hence, let's optimize the prepare step to avoid a heap allocation
in the common case.
Also, because we use nm_utils_g_main_context_create_integrate_source()
with the internal GMainContext of NMClient. That context always has
a small number of file descriptors to track and it doesn't see much
change. In the vast majority of cases, the heap allocation can be
avoided.
NMTernary is part of libnm's public API. It thus cannot be used by code
without libnm/libnm-core dependency.
Add another enum with the same purpose.
The name "NMTernary" is already taken, and we should not use some macro
trickery to use (effectively) different types under the same name.
Another possible name would be "NMTern", but for no strong reasons
we choose NMOptionBool. The naming reminds of rust's std::option::Option.
"src/nm-logging.c" should be independent of libnm-core. It almost
is, except the error domain and code.
Move NM_MANAGER_ERROR to "nm-glib-aux/nm-shared-utils.h" so that
"nm-logging.c" is independent of libnm-core.
This is the same as libnm's nm_utils_hwaddr_aton(), which however
is public API.
We want to use this function also without libnm(-core). Hence add
the helper to "shared/nm-glib-aux".
Enums can also be negative (contrary to Flags). Fix the parsing.
$ nmcli connection modify "$PROFILE" connection.llmnr -1
Error: failed to modify connection.llmnr: invalid option '-1', use one of [default,no,resolve,yes].
In the vast majority of cases is the string for _nm_utils_enum_from_str_full()
short. As we duplicate it for stripping, prefer to clone it on the stack
with nm_strdup_maybe_a().
The change broke unit tests on 32 bit systems.
Change the code again to make it more similar to what it was
before. Now only on 64 bit systems there is any difference compared
to before. That makes it easier about reasoning for how the unit test
should be (in most cases, it is unchanged).
Fixes: 040c86f15c ('shared: avoid compiler warning for nm_utils_get_next_realloc_size() returning huge sizes')
On s390x (gcc-8.3.1-5.1.el8.s390x) the compiler warns that we don't
pass size larger than 2^63-1 to malloc. With LTO enabled, it is also
quite adamant in detecting that with nm_utils_get_next_realloc_size().
Optimally, we would disable this useless warning with "-Wno-alloc-size-larger-than",
but that seems not to work. So add a workaround in code :(
It's hard to actually workaround the warning while handling all kinds of
sizes. The only simple solution is to no handle such huge cases and only
assert.
In function 'nm_secret_mem_realloc',
inlined from '_nm_str_buf_ensure_size' at shared/nm-glib-aux/nm-shared-utils.c:5316:31:
shared/nm-glib-aux/nm-secret-utils.h:180:17: error: argument 1 value '18446744073709551615' exceeds maximum object size 9223372036854775807 [-Werror=alloc-size-larger-than=]
m_new = g_malloc(new_len);
^
shared/nm-glib-aux/nm-secret-utils.h: In function '_nm_str_buf_ensure_size':
/usr/include/glib-2.0/glib/gmem.h:78:10: note: in a call to allocation function 'g_malloc' declared here
gpointer g_malloc (gsize n_bytes) G_GNUC_MALLOC G_GNUC_ALLOC_SIZE(1);
^
lto1: all warnings being treated as errors
We cannot drop the reference count to zero while having
no lock. Otherwise, another thread might race doing
s = nm_ref_string_new("...");
nm_ref_string_unref(s);
and already successfully delete the instance.
Hitting this race should be rather difficult, especially because
we tend to use NMRefString only from one thread. But still, access
to global variables must be race free.
Fixes: 908fadec96 ('shared: add NMRefString')
The macro should require exactly 6 parameters (for the 6 bytes
of the address). On the other hand, we also should be able to
use a macro like
NM_ETHER_ADDR_INIT(NM_BRIDGE_GROUP_ADDRESS_DEF_BIN)
To get that work properly, we need to expand the variadic macro
once.
Also, cast the result to the struct type. With this, it can
not only be used for initialization, but also for assignment
and temporary variables.
We have:
- nm_utils_hashtable_cmp(): this does a full cmp of two hash
tables, with the intent to provide a stable sort order.
It thus takes a GCompareDataFunc() argument.
- nm_utils_hashtable_cmp_equal(): this is like nm_utils_hashtable_cmp(),
except that the caller won't get a compare value, only a boolean
value that indicates equality.
This was previously called nm_utils_hashtable_equal().
- nm_utils_hashtable_equal(): this takes a GEqualFunc function
for comparing the values for equality. It takes thus
a different kind of predicate, but otherwise is similar to
nm_utils_hashtable_cmp_equal().
This was previously called nm_utils_hash_table_equal().
Unify the naming of these functions.
