NMTST_SWAP() used memcpy() for copying the value, while NM_SWAP() uses
a temporary variable with typeof(). I think the latter is preferable.
Also, the macro is essentially doing the same thing.
I think it's preferable to use nm_clear_g_free() instead of
g_clear_pointer(, g_free). The reasons are not very strong,
but I think it is overall preferable to have a shorthand for this
frequently used functionality.
sed 's/\<g_clear_pointer *(\([^;]*\), *\(g_free\) *)/nm_clear_g_free (\1)/g' $(git grep -l g_clear_pointer) -i
The abbreviations "ns" and "ms" seem not very clear to me. Spell them
out to nsec/msec. Also, in parts we already used the longer abbreviations,
so it wasn't consistent.
The targets that involve the use of the `NetworkManager` library,
built in the `src` build file have been improved by applying a set
of changes:
- Indentation has been fixed.
- Set of objects used in targets have been grouped together.
- Aritificial dependencies used to group dependencies and custom
compiler flags have been removed and their use replaced with
proper dependencies and compiler flags to avoid any confussion.
The `nm-default.h` header is used widely in the code by many
targets. This header includes different headers and needs different
libraries depending the compilation flags.
A new set of `*nm_default_dep` dependencies have been created to
ease the inclusion of different directorires and libraries.
This allows cleaner build files and avoiding linking unnecessary
libraries so this has been applied allowing the removal of some
dependencies involving the linking of unnecessary libraries.
These are deprecated. Also, they are nowadays implemented as macros
that expand to
#define g_main_run(loop) g_main_loop_run(loop) GLIB_DEPRECATED_MACRO_IN_2_26_FOR(g_main_loop_run)
This can cause compilation failure (in some environments).
If the @append_force argument is set and the object is already in the
list, it must be moved at the end.
Fixes: 22edeb5b69 ('core: track addresses for NMIP4Config/NMIP6Config via NMDedupMultiIndex')
Add test to show a wrong result of ip_ipX_config_replace() due to a
bug in _nm_ip_config_add_obj(). When an address is added to the tail
of the index and another address with the same id already exists, the
existing object is left at the same place, breaking the order of
addresses.
We no longer add these. If you use Emacs, configure it yourself.
Also, due to our "smart-tab" usage the editor anyway does a subpar
job handling our tabs. However, on the upside every user can choose
whatever tab-width he/she prefers. If "smart-tabs" are used properly
(like we do), every tab-width will work.
No manual changes, just ran commands:
F=($(git grep -l -e '-\*-'))
sed '1 { /\/\* *-\*- *[mM]ode.*\*\/$/d }' -i "${F[@]}"
sed '1,4 { /^\(#\|--\|dnl\) *-\*- [mM]ode/d }' -i "${F[@]}"
Check remaining lines with:
git grep -e '-\*-'
The ultimate purpose of this is to cleanup our files and eventually use
SPDX license identifiers. For that, first get rid of the boilerplate lines.
nmtst_get_rand_int() was originally named that way, because it
calls g_rand_int(). But I think if a function returns an uint32, it
should also be named that way.
Rename.
Don't create a heap allocated GString to hold the static
result of nm_logging_all_domains_to_string().
Instead, use a static buffer of the exactly required size.
The main reason to do this, is to get the exact size of
"_all_logging_domains_to_str" buffer. This is the upper
boundary for the size of a string buffer to hold all domain
names.
We will need that boundary in the next commit. The attractive
thing here is that we will have a unit-test failure if this
boundery no longer matches (--with-more-asserts). That means,
this boundary is guarded by unit tests and we don't accidentally
get it wrong when the domains change.
Also, take care to initialize the buffer in a thread-safe manner.
It's easy enough to get right, so there is no excuse for having
non-thread-safe code in logging.
The library is called "libnm_core". So the dependency should be called
"libnm_core_dep", like in all other cases.
(cherry picked from commit c27ad37c27)
The defaults for test timeouts in meson is 30 seconds. That is not long
enough when running
$ NMTST_USE_VALGRIND=1 ninja -C build test
Note that meson supports --timeout-multiplier, and automatically
increases the timeout when running under valgrind. However, meson
does not understand that we are running tests under valgrind via
NMTST_USE_VALGRIND=1 environment variable.
Timeouts are really not expected to be reached and are a mean of last
resort. Hence, increasing the timeout to a large value is likely to
have no effect or to fix test failures where the timeout was too rigid.
It's unlikely that the test indeed hangs and the increase of timeout
causes a unnecessary increase of waittime before aborting.
The test should check the behavior with "const typeof(a)" in a macro,
where "a" itself is const. For that we don't need a double const
declaration of v2.
Also, that fixes an actual compiler warning:
../src/tests/test-general.c: In function ‘test_duplicate_decl_specifier’:
../src/tests/test-general.c:1669:8: warning: duplicate ‘const’ declaration specifier [-Wduplicate-decl-specifier]
const const int v2 = 3;
^~~~~
NetworkManager is single-threaded and uses a mainloop.
However, sometimes we may need multiple threads. For example, we will
need to write sysctl values asynchronously, using the glib thread-pool.
For that to work, we also need to switch the network-namespace of the
thread-pool thread. We want to use NMPNetns for that. Hence it's better
to have NMPNetns thread-safe, instead of coming up with a duplicate
implementation. But NMPNetns may want to log, so we also need nm-logging
thread-safe.
In general, code under "shared/nm-utils" and nm-logging should be usable
from multiple threads. It's simpler to make this code thread-safe than
re-implementing it. Also, it's a bad limitation to be unable to log
from other threads. If there is an error, the best we can often do is to
log about it.
Make nm-logging thread-safe. Actually, we only need to be able to log
from multiple threads. We don't need to setup or configure logging from
multiple threads. This restriction allows us to access logging from the
main-thread without any thread-synchronization (because all changes in
the logging setup are also done from the main-thread).
So, while logging from other threads requires a mutex, logging from the
main-thread is lock-free.
The only purpose of using alloca() to avoid the overhead of heap-allocation
and possible save a line in source code for managing/freeing the heap allocation.
For tests we don't care about performance, and (in this case)
the code does not get any shorter.
Avoid alloca() in tests, because alloca() is something to search for
when reviewing code for stack overflows. No need to have such false
positives show up in tests.
Add a version of nm_utils_strbuf_append_*() that does not care
about NUL terminate strings, but accept any binary data. That makes
it useful for writing a binary buffer.
glib has an base64 implementation, but g_base64_decode() et al. gives
no way to detect invalid encodings. All invalid codes are silently
ignored. That is not suitable for strictly validating user input.
Instead of reimplementing of copy-pasting the code from somewhere,
reuse systemd's unbase64mem().
But don't use "hexdecoct.h" directly. Instead, add a single accessor
function to our "nm-sd-utils-shared.h" gateway. We want to be careful
about which bits from systemd we use, because otherwise re-importing
systemd code becomes fragile as you don't know which relevant parts
changed.
For better or worse, we already pull in large parts of systemd sources.
I need a base64 decode implementation (because glib's g_base64_decode()
cannot reject invalid encodings). Instead of coming up with my own or
copy-paste if from somewhere, reuse systemd's unbase64mem().
But for that, make systemd's basic bits an independent static library
first because I will need it in libnm-core.
This doesn't really change anything except making "libnm-systemd-core.la"
an indpendent static library that could be used from "libnm-core". We
shall still be mindful about which internal code of systemd we use, and only
access functionality that is exposed via "systemd/nm-sd-utils-shared.h".
If the spec specifies only negative matches (and none of them matches),
then the result shall be positive.
Meaning:
[connection*] match-device=except:dhcp-plugin:dhclient
[connection*] match-device=except:interface-name:eth0
[.config] enabled=except:nm-version:1.14
should be the same as:
[connection*] match-device=*,except:dhcp-plugin:dhclient
[connection*] match-device=*,except:interface-name:eth0
[.config] enabled=*,except:nm-version:1.14
and match by default. Previously, such specs would never yield a
positive match, which seems wrong.
Note that "except:" already has a special meaning. It is not merely
"not:". That is because we don't support "and:" nor grouping, but all
matches are combined by an implicit "or:". With such a meaning, having
a "not:" would be unclear to define. Instead it is defined that any
"except:" match always wins and makes the entire condition to explicitly
not match. As such, it makes sense to treat a match that only consists
of "except:" matches special.
This is a change in behavior, but the alternative meaning makes
little sense.
Add a new CON_DEFAULT() macro that places a property name into a
special section used at runtime to check whether it is a supported
connection default.
Unfortunately, this mechanism doesn't work for plugins so we have to
enumerate the connection defaults from plugins in the daemon using
another CON_DEFAULT_NOP() macro.
We already had "${DEVICE}" which uses the interface name.
In times of predictable interface naming, that works well.
It allows the user to generate IDs per device which don't
change when the hardware is replaced.
"${MAC}" is similar, except that is uses the permanent MAC
address of the device. The substitution results in the empty
word, if the device has no permanent MAC address (like software
devices).
The per-device substitutions "${DEVICE}" and "${MAC}" are especially
interesting with "connection.multi-connect=multiple".
Our internal DHCP client (from systemd) defaults to a particular client ID.
It is currently exposed as nm_sd_utils_generate_default_dhcp_client_id()
and is based on the systemd implementation.
One problem with that is, that it internally looks up the interface name
with if_indextoname() and reads /etc/machine-id. Both makes it harder
for testing.
Another problem is, that this way of generating the client-id is
currently limited to internal client. Why? If you use dhclient plugin,
you may still want to use the same algorithm. Also, there is no explict
"ipv4.dhcp-client-id" mode to select this client-id (so that it could
be used in combination with "dhclient" plugin).
As such, this code will be useful also aside systemd DHCP plugin.
Hence, the function should not be obviously tied to systemd code.
The implementation is simple enough, and since we already have a
unit-test, refactor the code to our own implementation.
Internal DHCP client generates a default client ID. For one,
we should ensure that this algorithm does not change without
us noticing, for example, when upgrading systemd code. Add
a test, that the generation algorithm works as we expect.
Also note, that the generation algorithm uses siphash24().
That means, siphash24() implementation also must not change
in the future, to ensure the client ID doesn't change. As we
patch systemd sources to use shared/c-siphash, this is not
obviously the case. Luckily c-siphash and systemd's siphash24 do
agree, so all is good. The test is here to ensure that.
Also, previously the generation algorithm is not exposed as a
function, sd_dhcp_client will just generate a client-id when
it needs it. However, later we want to know (and set) the client
id before starting DHCP and not leave it unspecified to an
implementation detail.
This patch only adds a unit-test for the existing DHCP client
ID generation to have something for comparison. In the next
commit this will change further.
