nm_keyfile_read() and nm_keyfile_write() will be public API.
As such, it must be flexible and extendible for future needs.
There is already the handler callback that fully solves this
(e.g. a future handler event could request whether a certain
behavior is enabled or not).
As additional possibility for future extension, add a flags
argument. Currently no flags are implemented.
For introspection/bindings it is cumbersome to access the
fields of the NMKeyfileHandlerData struct. Instead add accessor
functions.
Also, we wouldn't want to expose the struct in public API directly,
because it makes it harder to extend it without breaking ABI.
From inside a callback 4 properties are potentially interesting
to all callbacks: the currenty group, key, setting and property-name.
Refactor the code to track these properties in NMKeyfileHandlerData
and distinguish between the property name and the keyfile key.
When an error gets set, we should abort right away. We should
not come into a situation where we would try to emit another warning.
Don't check for a condition that should never happen and assert
against it.
Setting the error on the callback does not work well from bindings.
Instead, let bindings call a (future) nm_keyfile_handler_data_fail_with_error()
function on the handler_data to indicate failure.
As the keyfile handler callback will become public API, it needs to be
usable via bindings. A plain void pointer is not usable. Instead, add
a new type that can be used via introspection.
This will become public API. The enum for read and write callback
serves very similar purposes. Merge them so that we have fewer
types in the public API.
S390 options are stored in a separate [ethernet-s390-options] section.
This group must not be interpreted as a NMSetting name, otherwise we
log a bogus warning:
<warn> [1590523563.7757] keyfile: ethernet-s390-options: invalid setting name 'ethernet-s390-options'
Fixes: cf9b8d3bad ('libnm/keyfile: implement ethernet.s390-options in keyfile')
We are going to expose some of this API in libnm.
The name "gendata" (for "generic data") is not very suited. Instead,
call the public API nm_setting_option_*(). This also brings no naming
conflict, because currently no API exists with such naming.
Rename the internal API, so that it matches the API that we are going
to expose next.
We already have NMEthtoolID to handle coalesce options in a way that is
convenient programmatically. That is, we can iterate over all valid
coalesce options (it's just an integer) and use that in a more generic
way.
If NMEthtoolCoalesceState names all fields explicitly, we need explicit
code that names each coalesce option. Especially since NMEthtoolCoalesceState
is an internal and intermediate data structure, this is cumbersome
and unnecessary.
Thereby it also fixes the issue that nm_platform_ethtool_init_coalesce() has a
NMPlatform argument without actually needing it.
nm_platform_ethtool_init_coalesce() does not operate on a NMPlatform
instance, and should not have the appearance of being a method of
NMPlatform.
When configuring with sanitizers enabled, ./configure.ac sets
-DVALGRIND=1 in the CFLAGS.
This causes a compilation error later:
$ /bin/sh ./libtool --tag=CC --mode=compile gcc ... -DVALGRIND=1 ... src/dhcp/nm-dhcp-nettools.c
...
In file included from src/dhcp/nm-dhcp-nettools.c:16:
./shared/systemd/sd-adapt-shared/nm-sd-adapt-shared.h:73: error: "VALGRIND" redefined [-Werror]
#define VALGRIND 0
When parsing user input if is often convenient to allow stripping whitespace.
Especially with escaped strings, the user could still escape the whitespace,
if the space should be taken literally.
Add support for that to nm_utils_buf_utf8safe_unescape().
Note that this is not the same as calling g_strstrip() before/after
unescape. That is, because nm_utils_buf_utf8safe_unescape() correctly
preserves escaped whitespace. If you call g_strstrip() before/after
the unescape, you don't know whether the whitespace is escaped.
We want to use the function to unescape (compress) secrets. As such, we want
to be sure that no secrets are leaked in memory due to growing the buffer with
realloc. In fact, reallocation should never happen. Assert for that.
As reallocation cannot happen, we could directly fill a buffer with
API like nm_utils_strbuf_*(). But NMStrBuf has low overhead even in this
case.
Often it is useful to not accept %NULL as input argument, to catch bugs.
For simple functions like nm_ethtool_id_get_by_name(), such limitations
are more annoying than helpful. Gracefully accept %NULL and treat is
like an invalid ethtool option.
_ASSERT_data() checks static, immutable data. Even with more asserts enabled,
there is no need to do that every time. Use NM_MORE_ASSERT_ONCE().
Note that NM_MORE_ASSERT_ONCE() will return constant FALSE, when build
without a sufficiently high assertion level. That means, the compiler
will just optimize the rest away.
We have nm_str_not_empty() which is the inverse of that. The purpose
of nm_str_not_empty() is to normalize a string to either return
%NULL or a non-empty string, like
const char *
get_name (Object *obj)
{
return nm_str_not_empty (obj->name);
}
Sometimes, we however want to check whether a string is not empty.
So, we previously had two choices:
1) use a temporary variable:
const char *tmp;
tmp = get_string ();
if (tmp && tmp[0])
...
The problem with this variant is that it's more verbose (by requiring a
temporary variable). Another downside is that there are multiple ways
how to check for an empty string (!tmp[0], tmp[0] == '\0', !strlen (tmp),
strlen (tmp) == 0), and sure enough they are all in use.
2) use !nm_str_not_empty(). But this double negation looks really odd
and confusing.
Add nm_str_is_empty() instead.
nm_g_error_matches() can be inlined and first checks whether the error
argument is not NULL. At least from the keyfile accessor functions, use
this macro, as they are called many times.
Macros preferably behave function-like, for example in that they evaluate
arguments exactly ones. Sometimes, we want to evaluate arguments
lazily, like in NM_IN_SET() or nm_g_set_error_take_lazy(). But it
is almost always undesirable to evaluate an argument more than once.
Fix NM_STR_HAS_PREFIX() for that.
Also, rename the local variable to not use the name "_str",
which may be a common name that the caller would like to use.
GPtrArray does not support NULL terminating the pointer array. That
makes it cumbersome to use it for tracking a strv array. Add a few
helper functions nm_strvarray_*() that help using a GArray instead.
We had three callers of nm_keyfile_plugin_kf_get_integer_list(). Two
only wanted to read values in range of guint8. One, wanted to read
unsigned integers (for which nm_keyfile_plugin_kf_get_integer_list()
was not suitable).
Instead, implement a integer list reader ourself.
One change is that g_key_file_get_integer_list() would accept list elements
with a number followed by a white space and garbage ([1]). We don't do that,
so there is a change in behavior here. That seems preferable, we don't
want to accept garbage.
The error reason text from the reader now also changes, and obviously we
no longer fail for integer values larger than G_MAXINT.
[1] c9bf247eb9/glib/gkeyfile.c (L4445)
