There is really no reason for this to be a macro. Our hash-related
helpers (like nm_hash_update_val()) are macros because they do some
shenigans to accept arguments of different (compile-time) types. But
the arguments for nm_hash_obfuscate_ptr() are well known and expected
of a certain form.
Note that with "-O2" some quick testing shows that the compiler no
longer inlines the function. But I guess that's fine, probably the
compiler knows best anyway.
... and nm_utils_fd_get_contents() and nm_utils_file_set_contents().
Don't mix negative errno return value with a GError output. Instead,
return a boolean result indicating success or failure.
Also, optionally
- output GError
- set out_errsv to the positive errno (or 0 on success)
Obviously, the return value and the output arguments (contents, length,
out_errsv, error) must all agree in their success/failure result.
That means, you may check any of the return value, out_errsv, error, and
contents to reliably detect failure or success.
Also note that out_errsv gives the positive(!) errno. But you probably
shouldn't care about the distinction and use nm_errno_native() either
way to normalize the value.
nm_utils_file_set_contents() is a re-implementation of g_file_set_contents(),
as such it returned merely a boolean success value.
It's sometimes interesting to get the native error code. Let the function
deviate from glib's original g_file_set_contents() and return the error code
(as negative value) instead.
This requires all callers to change. Also, it's potentially a dangerous
change, as this is easy to miss.
Note that nm_utils_file_get_contents() also returns an errno, and
already deviates from g_file_get_contents() in the same way. This patch
resolves at least the inconsistency with nm_utils_file_get_contents().
- add nm_c_list_elem_find_first() macro that takes a predicate
and returns the first match.
This macro has a non-function-like behavior, which we often try to
avoid because macros should behave like functions. In this case it's
however convenient, so let's do it.
Also, despite being non-function-like, it should be pretty hard to
use wrongly.
- rename nm_c_list_elem_find_first() to nm_c_list_elem_find_first_ptr().
When we build n-dhcp4 for NetworkManager we get a compiler warning.
This can also be reproduced by building n-dhcp4 alone:
$ CFLAGS='-Werror=declaration-after-statement' meson build && ninja -C build
...
[36/47] Compiling C object 'src/25a6634@@ndhcp4-private@sta/n-dhcp4-outgoing.c.o'.
FAILED: src/25a6634@@ndhcp4-private@sta/n-dhcp4-outgoing.c.o
ccache cc -Isrc/25a6634@@ndhcp4-private@sta -Isrc -I../src -Isubprojects/c-list/src -I../subprojects/c-list/src -Isubprojects/c-siphash/src -I../subprojects/c-siphash/src -Isubprojects/c-stdaux/src -I../subprojects/c-stdaux/src -fdiagnostics-color=always -pipe -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -std=c11 -g -D_GNU_SOURCE -Werror=declaration-after-statement -fPIC -fvisibility=hidden -fno-common -MD -MQ 'src/25a6634@@ndhcp4-private@sta/n-dhcp4-outgoing.c.o' -MF 'src/25a6634@@ndhcp4-private@sta/n-dhcp4-outgoing.c.o.d' -o 'src/25a6634@@ndhcp4-private@sta/n-dhcp4-outgoing.c.o' -c ../src/n-dhcp4-outgoing.c
../src/n-dhcp4-outgoing.c: In function ‘n_dhcp4_outgoing_new’:
../src/n-dhcp4-outgoing.c:63:9: error: ISO C90 forbids mixed declarations and code [-Werror=declaration-after-statement]
63 | static_assert(N_DHCP4_NETWORK_IP_MINIMUM_MAX_SIZE >= N_DHCP4_OUTGOING_MAX_PHDR +
| ^~~~~~~~~~~~~
Coverity says
CID 202453 (#1 of 1): Wrong sizeof argument (SIZEOF_MISMATCH)suspicious_sizeof:
Passing argument user_data of type gconstpointer and argument (gsize)nargs * 8UL /* sizeof (gconstpointer) */ to function g_slice_free1 is suspicious.
Let's pass instead the "data" pointer. It's the same, but maybe that
avoids the warning.
Coverity doesn't like us ignoring the return value, although
we really only care about the "p" output pointer.
Try casting the result to (void), maybe that silences Coverity.
- don't let no_auto_default_from_file() do any preprocessing of
the lines that it reads. It merely splits the lines at '\n'
and utf8safe-unescapes them.
This was previously duplicated also by NMConfigData's property
setter. We don't need to do it twice.
- sort the lines. This makes the entire handling O(n*ln(n)) instead
of O(n^2). Also, sorting effectively normalizes the content, and
it's desirable to have one true representation of what we write.
In particular when calling nm_utils_strv_sort() with a positive length
argument, then this is not a %NULL terminated strv arrary. That may mean
that it makes sense for the input array to contain %NULL strings.
Use a strcmp() function that accepts %NULL too.
While this is not used at the moment, I think nm_utils_strv_sort()
should accept %NULL strings beause otherwise it's a possibly unexpected
restriction of its API. The function should handle sensible input gracefully.
It is like strcmp(), but has a signature suitable for GCompareDataFunc.
This is necessary with nm_utils_ptrarray_find_binary_search()
to search a sorted strv array.
The fault is here really C, which doesn't allow inline static functions.
So, you need all kinds of slightly different flavors for the same
callbacks (with or without user-data).
Note that glib2 internally just casts strcmp() to GCompareDataFunc ([1]),
relying on the fact how arguments are passed to the function and
ignoring the additional user-data argument. But I find that really
ugly and probably not permissible in general C. Dunno whether POSIX
would guarantee for this to work. I'd rather not do such function
pointer casts.
[1] 0c0cf59858/glib/garray.c (L1792)
Using clock_gettime() directly is a bit inconvenient. We usually
want to combine the fields of struct timespec into one timestamp
(for example, in unit nanoseconds).
Add a helper function to do that.
nm_strndup_a() uses strncpy() because we want the behavior of clearing out
the memory after the first NUL byte. But that can cause a compiler warning:
CC src/settings/plugins/keyfile/libNetworkManager_la-nms-keyfile-utils.lo
In file included from ../../shared/nm-default.h:279,
from ../../src/settings/plugins/keyfile/nms-keyfile-utils.c:20:
In function ‘_nm_strndup_a_step’,
inlined from ‘nms_keyfile_loaded_uuid_is_filename’ at ../../src/settings/plugins/keyfile/nms-keyfile-utils.c:65:9:
../../shared/nm-glib-aux/nm-macros-internal.h:1661:3: error: ‘strncpy’ specified bound depends on the length of the source argument [-Werror=stringop-overflow=]
1661 | strncpy (s, str, len);
| ^~~~~~~~~~~~~~~~~~~~~
../../src/settings/plugins/keyfile/nms-keyfile-utils.c: In function ‘nms_keyfile_loaded_uuid_is_filename’:
../../src/settings/plugins/keyfile/nms-keyfile-utils.c:48:8: note: length computed here
48 | len = strlen (filename);
| ^~~~~~~~~~~~~~~~~
It's true that the len argument of _nm_strndup_a_step() depends on the
string length of the source string. But in this case it's safe, because
we checked that the destination buffer is exactly the right size too.
By that reasoning we should use memcpy() or strcpy(), but both are
unsuitable. That is because we want nm_strndup_a() to behave like
strndup(), which means we need to handle cases where the len argument
is larger than the string length of the source string. That is, we want
always to return a buffer of size len+1, but we want to copy only the
characters up to the first NUL byte, and clear out the rest. That's what
strncpy() does for us.
Silence the warning.
If there is only one argument, we can assume this is a plain string.
That is especially the case, because g_set_error() is G_GNUC_PRINTF()
and would warn if this would be a format string with missing parameters.
This is for convenience. Previously, one was compelled to explicitly
choose between nm_utils_error_set_literal() and nm_utils_error_set().
Now, it automatically chooses.
Note that there are a few things that won't work, like
nm_utils_error_set (error, code, "bogus %u escape");
But that's good. You get a compiler warning (as you used to)
and it's clear in this case you really need
nm_utils_error_set_literal().
This follows a pointer to a pointer and compares them. In a sense
it's like nm_pstr_*(), which follow a pointer to a string. However,
these functions use direct pointer comparison.
The purpose is when you hash a key that has as first field a pointer
value (and then compare them by pointer equality).
The probe takes a reference to the current lease and so it must
release it upon destruction.
Signed-off-by: Beniamino Galvani <bgalvani@redhat.com>
https://github.com/nettools/n-dhcp4/pull/1
Add nm_sd_dns_name_to_wire_format() based on systemd utilities to
convert a name into its wire format according to RFC 1035 section
3.1. It will be used to build the content of the DHCP FQDN option.
dns-domain.c contains useful functions for manipulating DNS names.
Add it to the systemd static library we build in shared/, similarly to
what we already do for other utility files that were originally in
src/systemd/src/basic/.
In particular, avoid including linux/netdevice.h from headers. This is
not a problem on newer distros, but required for CentOS 7.6.
Signed-off-by: Tom Gundersen <teg@jklm.no>
This is inspired by the existing systemd integration, with a few differences:
* This parses the WPAD option, which systemd requested, but did not use.
* We hook into the DAD handling, only making use of the configured address
once DAD has completed successfully, and declining the lease if it fails.
There are still many areas of possible improvement. In particular, we need
to ensure the parsing of all options are compliant, as n-dhcp4 treats all
options as opaque, unlike sd-dhcp4. We probably also need to look at how
to handle failures and retries (in particular if we decline a lease).
We need to query the current MTU at client startu, as well as the hardware
broadcast address. Both these are provided by the kernel over netlink, so
it should simply be a matter of hooking that up with NM's netlink layer.
Contribution under LGPL2.0+, in addition to stated licenses.
Previously, nm_c_list_move_*() only allowed to move element inside the
same list. Relax that, it works just the same list to move the element
from one list into a different list.
