g_steal_pointer() as provided by glib improved significantly. Nowadays it
casts the return type via the non-standard typeof() operator.
But this useful feature is only enabled with
GLIB_VERSION_MAX_ALLOWED >= GLIB_VERSION_2_58
which NetworkManager does not set.
This macro is hardly rocket science. Always provide our own
implementation, that always does the casting (we rely on gcc/clang
to support typeof() already at many places).
A helper method, only useful for printf debugging -- and thus
unused in the source-tree.
It is relatively cumbersome to lookup the GType that implements
a property. For example, for NMDeviceBond.driver, it should return
NMDevice (which implements the "driver" property).
There is no advantage in having these as macros. Make them
inline functions, compiler should be able to decide that they
are in fact inlinable.
Also, don't call g_strcmp0() for nm_streq0(). It means we first
have to call glib function, only to call a glibc function. No need
for this abstraction.
Contrary to g_str_has_suffix(), it exploits the fact the the suffix length
is known at compile time. No need to call a glib function, to find out what
we already know, to call strcmp().
Instead just calculate the string length and call memcmp().
Subsequent calls to nm_strerror_native() overwrite the previous
buffer. That is potentially dangerious. At least functions in
shared/nm-utils (which are lower-layer utilities) should not do
that and instead use a stack-local buffer. That is because these
functions should not make assumptions about the way they are called.
On the other end, nmcli passing the return-value of nm_strerror_native()
to g_print() is clearly OK because the higher layers are in control of
when the call nm_strerror_native() -- by relying that lower layers don't
interfere.
We have various options for strerror(), with ups and downsides:
- strerror()
- returns pointer that is overwritten on next call. It's convenient
to use, but dangerous.
- not thread-safe.
- not guaranteed to be UTF-8.
- strerror_r()
- takes input buffer and is less convenient to use. At least, we
are in control of when the buffer gets overwritten.
- there is a Posix/XSI and a glibc variant, making it sligthly
inconvenient to used. This could be solved by a wrapper we implement.
- thread-safe.
- not guaranteed to be UTF-8.
- g_strerror()
- convenient and safe to use. Also the buffer is never released for the
remainder of the program.
- passing untrusted error numbers to g_strerror() can result in a
denial of service, as the internal buffer grows until out-of-memory.
- thread-safe.
- guaranteed to be UTF-8 (depending on locale).
Add our own wrapper nm_strerror_native(). It is:
- convenient to use (returning a buffer that does not require
management).
- slightly dangerous as the buffer gets overwritten on the next call
(like strerror()).
- thread-safe.
- guaranteed to be UTF-8 (depending on locale).
- doesn't keep an unlimited cache of strings, unlike g_strerror().
You can't have it all. g_strerror() is leaking all generated error messages.
I think that is unacceptable, because it would mean we need to
keep track where our error numbers come from (and trust libraries we
use to only set a restricted set of known error numbers).
Use the NM_ERRNO_NATIVE() macro that asserts that these errno numbers are
indeed positive. Using the macro also serves as a documentation of what
the meaning of these numbers is.
That is often not obvious, whether we have an nm_errno(), an nm_errno_native()
(from <errno.h>), or another error number (e.g. WaitForNlResponseResult). This
situation already improved by merging netlink error codes (nle),
NMPlatformError enum and <errno.h> as nm_errno(). But we still must
always be careful about not to mix error codes from different
domains or transform them appropriately (like nm_errno_from_native()).
The native error numbers (from <errno.h>) and our nmerr extention on top
of them are almost the same. But there are peculiarities.
Both errno and nmerr must be positive values. That is because some API
(systemd) like to return negative error codes. So, a positive errno and
its negative counter part indicate the same error. We need normalization
functions that make an error number positive (these are nm_errno() and
nm_errno_native()).
This means, G_MININT needs special treatment, because it cannot be
represented as a positive integer. Also, zero needs special
treatment, because we want to encode an error, and zero already encodes
no-error. Take care of these special cases.
On top of that, nmerr reserves a range within native error numbers for
NetworkManager specific failure codes. So we need to transition from native
numbers to nmerr numbers via nm_errno_from_native().
Take better care of some special cases and clean them up.
Also add NM_ERRNO_NATIVE() macro. While nm_errno_native() coerces a
value in the suitable range, NM_ERRNO_NATIVE() asserts that the number
is already positive (and returns it as-is). It's use is only for
asserting and implicitly documenting the requirements we have on the
number passed to it.
NMIPAddr contains an unnamed union. We have to either explicitly
initialize one field, or omit it.
../shared/nm-utils/nm-shared-utils.c:38:36: error: suggest braces around initialization of subobject [-Werror,-Wmissing-braces]
const NMIPAddr nm_ip_addr_zero = { 0 };
^
{}
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.
There is:
1) glib's MAX() macro, which evaluates arguments multiple times,
but yields a constant expression, if the arguments are constant.
2) NM's NM_MAX() macro, which evaluates arguments exactly once,
but never yields a constant expression.
3) systemd's MAX() which is like NM_MAX().
Now, it's sensible to use
char buf[MAX (A_CONSTANT, ANOTHER_CONSTANT)];
and this works with glib's variant (1).
However, when we include systemd headers, 1) gets redefined to 3), and
above no longer works. That is because we we don't allow VLA and systemd's
macro gives not a constant expression.
Add NM_CONST_MAX() macro which is like systemd's CONST_MAX(). It can
only operate on constant arguments.
NMIPAddr is a union of IPv4 and IPv6 addresses.
A lot of our internal API handles IPv4 as in_addr_t / guint32 / be32_t
types, as such the union field "addr4" is just a plain number. Possibly
the internal API should be all refactored to prefer "struct in_addr"
instead, but that is yet to be done.
Anyway, at a few places we will need also access to the IPv4 address in form of
a `struct in_addr`. Add an alias for that.
I am not too happy about the resulting naming. It would be nicer to have
struct in_addr addr4;
struct in6_addr addr6;
in_addr_t s_addr4;
but for now, don't do such renaming.
NM_UTILS_LOOKUP_STR() uses alloca(). Partly to avoid the overhead of
malloc(), but more important because it's convenient to use. It does
not require to declare a varible to manage the lifetime of the heap
allocation.
It's quite safe, because the stack allocation is of a fixed size of only
a few bytes. Overall, I think the convenience that we get (resulting in
simpler code) outweighs the danger of stack allocation in this case. It's
still worth it.
However, as it uses alloca(), it still must not be used inside a (unbound)
loop and it is obviously a macro.
Rename the macros to have a _A() suffix. This should make the
peculiarities more apparent.
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.
Since we already cached the result of getpagesize() in a static variable (at
two places), move the code to nm-shared-utils, so it is reusable.
Also, use sysconf() instead of getpagesize(), like suggested by `man
getpagesize`.
Using strncpy() in the macro directly can result in a compiler warning.
We don't want to replace this with memcpy(), because strncpy() aborts
on the first NUL and fills the rest with NUL. Since nm_strndup_a() is a
replacement for g_strndup(), we want to do that here as well.
In file included from ../shared/nm-default.h:294,
from ../libnm-core/nm-utils.c:22:
../libnm-core/nm-utils.c: In function nm_sock_addr_endpoint_new:
../shared/nm-utils/nm-shared-utils.h:281:4: error: strncpy output truncated before terminating nul copying as many bytes from a string as its length [-Werror=stringop-truncation]
strncpy (_s, _str, _len); \
^~~~~~~~~~~~~~~~~~~~~~~~
../libnm-core/nm-utils.c:154:26: note: in expansion of macro nm_strndup_a
host = _parse_endpoint (nm_strndup_a (200, endpoint, l_endpoint - 1, &host_clone),
^~~~~~~~~~~~
../libnm-core/nm-utils.c:152:15: note: length computed here
l_endpoint = strlen (endpoint) + 1;
^~~~~~~~~~~~~~~~~
Add helper wrappers around nm_g_object_set_property() that take a
native value, construct a GValue of the according type, and call
nm_g_object_set_property().
Commonly, the prefix is a string constant. We don't need to call
g_str_has_prefix() for that, which first requires strlen() on
the prefix. All the information is readily available.
Add a macro for that.
